#14799 closed defect (fixed)
reduce 'port lint' pedantry
Reported by: | ghosthound | Owned by: | jmpalacios (Juan Manuel Palacios) |
---|---|---|---|
Priority: | Normal | Milestone: | MacPorts 1.7.0 |
Component: | base | Version: | 1.7.0 |
Keywords: | Cc: | ryandesign (Ryan Carsten Schmidt), blb@… | |
Port: |
Description
Attached patch reduces 'port lint' pedantry by silencing the following:
whitespace checks (except for newline at EOF). They're invisible to port and to humans.
patchfile naming needs only have 'patch-', no other requirements.
Attachments (4)
Change History (17)
Changed 17 years ago by ghosthound
Attachment: | patch-src_port1.0_portlint.tcl added |
---|
comment:1 Changed 17 years ago by raimue (Rainer Müller)
We should discuss the patch naming policy on the list before making changes. I don't know when or how it was decided to be 'patch-*.diff', but why change it to 'patch-*' now and not '*.diff'?
I think this was introduced to make it easier for committers to use patches with their favorite editor. Therefore '*.diff' would match this requirement more than 'patch-*'.
Also, I think this might have reference at other places (the guide?), where it would have to be changed, too.
comment:2 Changed 17 years ago by ryandesign (Ryan Carsten Schmidt)
Cc: | ryandesign@… added |
---|
This is not to be implemented until discussion on the mailing list concludes and a decision has been reached.
Changed 17 years ago by ryandesign (Ryan Carsten Schmidt)
Attachment: | portlint.tcl.patchfile-name.diff added |
---|
be less pedantic about patchfile names
Changed 17 years ago by ryandesign (Ryan Carsten Schmidt)
Attachment: | portlint.tcl.blank-lines.diff added |
---|
don't warn about missing blank lines
Changed 17 years ago by ryandesign (Ryan Carsten Schmidt)
Attachment: | portlint.tcl.trailing-whitespace.diff added |
---|
only warn about trailing whitespace after a backslash
comment:3 follow-up: 4 Changed 17 years ago by wsiegrist@…
The server's portlint.tcl has been updated to the latest. This fixes some bugs from v1.6, however, it completely breaks patchfile name warnings as a "file exists" check was added to fix warning about upstream patches. Auto linting does not have access to the files/ directory, so it thinks all patches are remote since they dont exist. Once a final decision/patch is decided on for portlint, the patchfile exists issue should be included in that.
comment:4 follow-up: 5 Changed 17 years ago by ryandesign (Ryan Carsten Schmidt)
Replying to wsiegrist@apple.com:
it completely breaks patchfile name warnings as a "file exists" check was added to fix warning about upstream patches. Auto linting does not have access to the files/ directory, so it thinks all patches are remote since they dont exist.
Didn't you cause this issue by making the server's port lint checkout non-recursive in r35474?
comment:5 Changed 17 years ago by wsiegrist@…
Owner: | changed from jberry@… to jmpp@… |
---|
Replying to ryandesign@macports.org:
Didn't you cause this issue by making the server's port lint checkout non-recursive in r35474?
The lint process was always non-recursive to minimize the work done by the server. If "file exists" is the only way to enforce the naming policy then I'll fix it, but like I said in my last comment, I am holding off on any further changes until the policy is decided upon. In effect, this disables patch file name errors completely until a decision is made.
I'm going to assign to jmpp for a portmgr decision on this.
comment:6 Changed 17 years ago by jmpalacios (Juan Manuel Palacios)
Component: | ports → base |
---|
As I said in my latest post to the dev list, we should:
"not advise on patchfile naming in any way (either if that's patch-*.diff, patch-*, *.diff or *.patch) at the lint level. From the very beginning this was only a suggestion that most of us agreed on, so we can still keep it in the guide and elsewhere as such, a suggestion; but it was just that, a suggestion, so I really don't think we have any business annoying a maintainer who, after reading the guide and what-not, has still chosen to name his patches in whatever other way. So my take on this particular issue is that we should remove all patch naming checks from lint, and as a direct result any [file exists foo] checks, which would instantly resolve Bill's comments here."
So lets wait a little while in case I have to deal with any flames on the list, but otherwise I'd consider the non-recursive nature of the checkouts and no resulting access to patchfiles a non-issue.
-jmpp
comment:7 Changed 17 years ago by jmpalacios (Juan Manuel Palacios)
Milestone: | MacPorts base bugs → MacPorts 1.6.1 |
---|
comment:8 Changed 17 years ago by afb@…
It would be great if someone could add a whitespace option to the "lint" command, so that one can actually use the tool to check the Portfile even if the autonag email doesn't complain anymore...
comment:9 Changed 17 years ago by afb@…
like in portlint(1) options:
-t Nit pick about use of spaces.
comment:10 Changed 16 years ago by blb@…
Cc: | blb@… added |
---|
How much of this should go into 1.7.0? Everything but afb's nit-picky-spacey suggestion appears to have patches (though they may not apply cleanly now).
comment:11 Changed 16 years ago by ryandesign (Ryan Carsten Schmidt)
As of r41170, lint no longer nitpicks whitespace or patchfile names. I think this is sufficient for 1.7.0.
I defined a new variable "nitpick" set to false. There is no exposed mechanism to set it to true. The last step would be to provide a way to set nitpick to true via some command-line switch. Do you know how to do that? I'm not sure I do.
comment:12 Changed 16 years ago by blb@…
Resolution: | → fixed |
---|---|
Status: | new → closed |
nitpick option added to 'port lint' in r41511.
$ port lint cidr ---> Verifying Portfile for cidr ---> 0 errors and 0 warnings found. $ port lint --nitpick cidr ---> Verifying Portfile for cidr Warning: Line 4 should be a newline (after PortSystem) ---> 0 errors and 1 warnings found.
reduce 'port lint' pedantry