Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-15742: Write shell script to serve as a Jenkins hook #46

Merged
merged 2 commits into from Sep 21, 2018

Conversation

kfindeisen
Copy link
Member

This PR introduces a script, run_ci_dataset.sh, which encapsulates the environment setup and data ID handling needed to run ap_verify on an entire dataset. By analogy with validate_drp/examples/processData.sh, the script assumes that the dataset(s) to be run are already downloaded and EUPS set up, and that uploading metric files is the responsibility of the .groovy script.

The first commit fixes a fatal bug in ap_verify that was, ironically, missed due to lack of CI.

This PR cannot be merged before #40, as the script relies on the latter PR's definition of CI-HiTS2015.

Copy link
Member

@jhoblitt jhoblitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without having actually run it, this looks completely reasonable and passes shellcheck 0.4.7.

Is the copyright boilerplate necessary?

@kfindeisen
Copy link
Member Author

I would think so. The rules technically only speak of "source" files, but the file is part of the package's codebase.

@jhoblitt
Copy link
Member

GPLv3 does not require a copyright notice per file. It encourages it in the "How to Apply These Terms to Your New Programs" section, which isn't part of the terms of the license. It does, however, forbid the removal of the boilerplate notices from derivative works. I have a strong preference to limit source files to technical matters and to use a simple LICENSE file at the repo/project root.

/cc @jonathansick

@timj
Copy link
Member

timj commented Sep 19, 2018

@jhoblitt the developer guide defines the policy for license and copyright.

@jonathansick
Copy link
Member

@jhoblitt Sorry, I had a brain freeze yesterday and was thinking of an earlier draft version of the policy that did minimize the boilerplate.

@jhoblitt
Copy link
Member

This statement in the dev guide is technically incorrect:

The GPL-3.0 license requires each source file to have a preamble comment containing a license statement. This is the generic license preamble:

If it is required, it is only by DM policy.

@timj
Copy link
Member

timj commented Sep 19, 2018

FSF strongly prefer that the license preamble is in the file. Why would we go against their wishes? I'm fine with you changing the dev guide to stop saying "requires" and instead say "strongly encourages" and that DM policy is to follow their advice.

@jhoblitt
Copy link
Member

Because it liters every file with boilerplate, many of which probably don't even qualify as a copyrightable work? GNU wants people to use https://www.gnu.org/prep/standards/standards.html as well; not everything GNU is a good idea.

@jdswinbank
Copy link
Contributor

Hey folks, I suggest GitHub isn't a great venue for this discussion. @jhoblitt, if you'd like to suggest a change to the rules, please file a ticket on Jira. Thanks!

@kfindeisen
Copy link
Member Author

This conversation has derailed a bit from my (admittedly implied) question: does a shell script count as a "source file" for the purposes of DM's copyright policy?

@jonathansick
Copy link
Member

@kfindeisen Yep.

ApPipeTask's constructor signature changed in DM-15096, but the
constructor call was never updated. This caused Python to complain
about multiple `config` arguments.
@kfindeisen kfindeisen merged commit 0cb4fd4 into master Sep 21, 2018
@kfindeisen kfindeisen deleted the tickets/DM-15742 branch September 21, 2018 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants