when testing the patch http://drupal.org/files/issues/sess_regen_hardening_280934-20a.patch
on minor level, I get 

Line 2: put a space between the asterisk and the comment text
/*/

but I cannot find the string /*/ in the patch.

What do line numbers correspond to? for instance I'm able to locate the line 7 (trailing space), but cannot see from which line the counting starts from.

CommentFileSizeAuthor
#2 coder_336230.patch3.19 KBstella
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stella’s picture

stella CreditAttribution: stella commented

The first is a known issue for patches with start with */.

As for line numbering, the line numbers given aren't the same as the file line numbers, rather they correspond to a line within a given hunk. Each hunk starts with a line something like @@ -7,6 +7,9 @@. This is because each hunk has to be reviewed separately from each other.

stella’s picture

stella CreditAttribution: stella commented
Status: Active » Needs review
FileSize
coder_336230.patch3.19 KB

Try this patch. It should fix both issues:

  • Problems when patch starts with a * or */.
  • Line numbers being off by 1 when reviewing a patch created with the -p option.
scor’s picture

scor CreditAttribution: scor commented

the patch works and fixes the problem with /*/.

however I realize that coder runs a code review on the actual changes of the patch (lines starting with +) and but also on the context of the changes as well. I'm not sure this is a good thing. When I review a patch, I mainly want to see if the code I introduce is valid, and old code issues might confuse me (noise). The coder patch feature should only review the code changes and ignore the older code. my .02 cts.

stella’s picture

stella CreditAttribution: stella commented

There's arguments for and against. The main one against your proposal (in my opinion) is that by not reviewing the surrounding lines, we loose context. We could end up reviewing some comment lines and then code lines, without ever actually processing the line that closes the comment block. That would break the reviews - unless we treated each individual set of + lines separately, which I think is too complicated to do for such little gain.

Doug: can you review before I commit it?

Cheers,
Stella

douggreen’s picture

douggreen CreditAttribution: douggreen commented
Version: 6.x-2.x-dev » 7.x-2.x-dev

I suspect this is still a problem?

douggreen’s picture

douggreen CreditAttribution: douggreen commented
Assigned: Unassigned » stella
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Issue summary: View changes
Status: Needs review » Closed (won't fix)

Coder 7.x is frozen now and will not receive updates. Coder 8.x-2.x can be used to check code for any Drupal version, Coder 8.x-2.x also supports the phpcbf command to automatically fix conding standard errors. Please check if this issue is still relevant and reopen against that version if necessary.