allow skipping sanity check#2549
Merged
damianam merged 4 commits intoeasybuilders:developfrom Aug 21, 2018
Merged
Conversation
…p, and is skippable via the 'skipsteps' easyconfig parameter
houndci-bot
reviewed
Aug 18, 2018
| # check log message with --skip for existing module | ||
| args = [ | ||
| eb_file, | ||
| toy_ec , |
Closed
067e12c to
79ecaae
Compare
79ecaae to
de96eeb
Compare
damianam
requested changes
Aug 20, 2018
| # skip step if specified as individual (skippable) step | ||
| if skippable and (self.skip or step in self.cfg['skipsteps']): | ||
| # under --skip, sanity check is not skipped | ||
| general_skip = self.skip and step != SANITYCHECK_STEP |
Member
There was a problem hiding this comment.
I find the name of this variable confusing. I think cl_skip would be more clear, since we are dealing the command line here.
damianam
approved these changes
Aug 21, 2018
Member
|
lgtm! |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change makes the sanity check step skippable, by including
'sanitycheck'inskipstepsin the easyconfig file, as is requested in #1626.Additional care was taken to make sure that the sanity check step is not skipped when using the
--skipcommand line option (as is currently the case already).I should mention that I very much advise against skipping the sanity check step, and that we will not accept easyconfigs in the central repository (https://github.com/easybuilders/easybuild-easyconfigs) that have
'sanitycheck'included inskipsteps.I may enhance this PR to spit out a big fat warning when the sanity check is being skipped, to make it clear that this is consider bad practice...
@easybuilders/easybuild-framework-maintainers Please provide some feedback on this change.