F/missing py license (Python, Perl and Bash scripts were missing declarations) #985
Conversation
bkil-syslogng
force-pushed
the
f/missing-py-license
branch
2 times, most recently
from
bee34d3
to
4bd9d6e
Compare
| @@ -0,0 +1,22 @@ | |||
| #!/usr/bin/env python | |||
| ############################################################################# | |||
There was a problem hiding this comment.
I don't think we need a shebang line here and neither of other modules in the same package.
There was a problem hiding this comment.
- Some of the Python files contained this, some of them did not. I wanted to make it uniform.
- I recommend using a shebang in each and every script to make file magic consistent on all platforms.
In light of these, do you agree?
There was a problem hiding this comment.
executable scripts contained this, whereas modules to be imported didn't.
having a shebang line indicates that the file would be executable, which
they are not.
Bazsi
On Mon, Mar 21, 2016 at 10:22 AM, Tamas Nagy notifications@github.com
wrote:
In modules/python/pylib/syslogng/init.py
#985 (comment):@@ -0,0 +1,22 @@
+#!/usr/bin/env python
+#############################################################################@bazsi https://github.com/bazsi
- Some of the Python files contained this, some of them did not. I wanted
to make it uniform.- I recommend using a shebang in each and every script to make file magic
consistent on all platforms.In light of these, do you agree?
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/balabit/syslog-ng/pull/985/files/4bd9d6e8f9bcba316d6ae8b02b701ffa4d85f6ea#r56795686
|
Apart from my nitpicks wrt shebang lines, this looks great, thank you. |
|
@bazsi I've added two new commits to remove redundant shebangs. Please notify me after you have reviewed them, so I can squash the fixup. |
|
ok on my part, thanks for the fixups. |
| @@ -22,6 +22,6 @@ modules/native | |||
| GPLv2+_SSL | |||
| tests | |||
| modules/java/(tools|[^/]*$) | |||
| modules/java-modules/(dummy|elastic|hdfs|http|kafka|[^/]*$) | |||
| modules/java-modules/(dummy|elastic|elastic-v2|hdfs|http|kafka|[^/]*$) | |||
There was a problem hiding this comment.
Hmm... we should add every new module to this file?
That will always be a problem...
There was a problem hiding this comment.
Yes, we do need to add each one when using the current format. This is because we have both GPL and non-GPL modules in the same folder, and I don't want to add overlapping patterns and row based prioritization, because that can bite us very quickly without ever noticing. As a side note, currently, overlapping declarations produce an error (e.g., the containing folder under GPL and subfolders under LGPL).
After these improvements are merged, this check (and Travis) should always tell you if you forgot to add a module and it should also point out which one. The omission was on my part that I forgot to check .java files, so that's why you didn't get a notification about it before.
|
@bkil-syslogng : could you squash the patches? |
Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
* Support parsing .ac, .am, .sh, .java, .pl and .py files * Ignore built ones * List further extensions to ignore * Fix handling regexp escaping when reading policy Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
bkil-syslogng
force-pushed
the
f/missing-py-license
branch
from
6cdbc2f
to
92e1067
Compare
|
Squashed and rebased to master. Backup saved: |
Also enabled automatic checking in the future for the most important non-C kinds (e.g., Java).