Skip to content

fix --try issues#1144

Merged
boegel merged 4 commits intoeasybuilders:developfrom
boegel:tweak_tweak
Jan 23, 2015
Merged

fix --try issues#1144
boegel merged 4 commits intoeasybuilders:developfrom
boegel:tweak_tweak

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Jan 23, 2015

  • stop adding annoying # tweaked comments
  • fix issue of disappearing newline
  • no adding of useless newline at the end

without this patch:

$ diff -u easybuild/easyconfigs/w/WRF/WRF-3.6.1-intel-2014b-dmpar.eb  /var/folders/8s/_frgh9sj6m744mxt5w5lyztr0000gn/T/easybuild-s0R3JD/tweaked_easyconfigs/WRF-3.6.1-intel-2015a-dmpar.eb
--- easybuild/easyconfigs/w/WRF/WRF-3.6.1-intel-2014b-dmpar.eb  2015-01-14 12:14:31.000000000 +0100
+++ /var/folders/8s/_frgh9sj6m744mxt5w5lyztr0000gn/T/easybuild-s0R3JD/tweaked_easyconfigs/WRF-3.6.1-intel-2015a-dmpar.eb    2015-01-23 15:21:00.000000000 +0100
@@ -5,8 +5,7 @@
 description = """The Weather Research and Forecasting (WRF) Model is a next-generation mesoscale
  numerical weather prediction system designed to serve both operational forecasting and atmospheric
  research needs."""
-
-toolchain = {'name': 'intel', 'version': '2014b'}
+toolchain = {'name': 'intel', 'version': '2015a'} # tweaked by EasyBuild (was: {'name': 'intel', 'version': '2014b'})
 toolchainopts = {'opt': False}  # don't use agressive optimization, stick to -O2

 sources = ['%(name)sV%(version)s.TAR.gz']

with this patch:

diff -u easybuild/easyconfigs/w/WRF/WRF-3.6.1-intel-2014b-dmpar.eb /var/folders/8s/_frgh9sj6m744mxt5w5lyztr0000gn/T/easybuild-08vVqj/tweaked_easyconfigs/WRF-3.6.1-intel-2015a-dmpar.eb 
--- easybuild/easyconfigs/w/WRF/WRF-3.6.1-intel-2014b-dmpar.eb  2015-01-14 12:14:31.000000000 +0100
+++ /var/folders/8s/_frgh9sj6m744mxt5w5lyztr0000gn/T/easybuild-08vVqj/tweaked_easyconfigs/WRF-3.6.1-intel-2015a-dmpar.eb    2015-01-23 16:38:22.000000000 +0100
@@ -6,7 +6,7 @@
  numerical weather prediction system designed to serve both operational forecasting and atmospheric
  research needs."""

-toolchain = {'name': 'intel', 'version': '2014b'}
+toolchain = {'name': 'intel', 'version': '2015a'}
 toolchainopts = {'opt': False}  # don't use agressive optimization, stick to -O2

 sources = ['%(name)sV%(version)s.TAR.gz']

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jan 23, 2015

@wpoely86: please review

wpoely86
wpoely86 reviewed Jan 23, 2015
View reviewed changes
Comment thread easybuild/framework/easyconfig/tweak.py
Copy link
Copy Markdown
Member

@wpoely86 wpoely86 Jan 23, 2015

Choose a reason for hiding this comment

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

shouldn't the first \s* not be outside the group?

Copy link
Copy Markdown
Member Author

@boegel boegel Jan 23, 2015

Choose a reason for hiding this comment

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

no, this is exactly what fixes the issue where the newline above toolchain was disappearing...

tweaking is not about making the easyconfigs adher to the style policy, so if there was whitespace before a key, leave it there

Copy link
Copy Markdown
Member

@wpoely86 wpoely86 Jan 23, 2015

Choose a reason for hiding this comment

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

wait, moving into named groups fixes the newline thing?

Copy link
Copy Markdown
Member Author

@boegel boegel Jan 23, 2015

Choose a reason for hiding this comment

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

No, including the \s* into the group, and using the group rather than hard replacing the key, that fixes it.

Copy link
Copy Markdown
Member

@wpoely86 wpoely86 Jan 23, 2015

Choose a reason for hiding this comment

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

a right

@fgeorgatos
Copy link
Copy Markdown
Contributor

fgeorgatos commented Jan 23, 2015

thank you for this; 1st visual review OK.

wpoely86
wpoely86 reviewed Jan 23, 2015
View reviewed changes
Comment thread easybuild/framework/easyconfig/tweak.py Outdated
Copy link
Copy Markdown
Member

@wpoely86 wpoely86 Jan 23, 2015

Choose a reason for hiding this comment

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

should there be this be

not (res.group('val') == val)

to increase readability?

Copy link
Copy Markdown
Member Author

@boegel boegel Jan 23, 2015

Choose a reason for hiding this comment

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

I'll use != instead

@hpcugentbot
Copy link
Copy Markdown

hpcugentbot commented Jan 23, 2015

Test FAILed.

@hpcugentbot
Copy link
Copy Markdown

hpcugentbot commented Jan 23, 2015

Test PASSed.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jan 23, 2015

Going in, thanks for the review @wpoely86, @fgeorgatos!

boegel added a commit that referenced this pull request Jan 23, 2015
@boegel
Merge pull request #1144 from boegel/tweak_tweak
90f52a2 
fix --try issues
@boegel boegel merged commit 90f52a2 into easybuilders:develop Jan 23, 2015
@boegel boegel deleted the tweak_tweak branch January 23, 2015 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@boegel @fgeorgatos @hpcugentbot @wpoely86