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

[formatv2] split off supported section from default section, add preliminary support for a dependencies section #826

Merged
merged 9 commits into from
Jan 30, 2014

Conversation

boegel
Copy link
Member

@boegel boegel commented Jan 29, 2014

@stdweird: review? it's missing a bit for which I'm counting on you (a parser for the the dependency 'value' string)

@ghost ghost assigned stdweird Jan 29, 2014
Returns None if translation to a dictionary is not possible (e.g. non-equals operator, missing version, ...).
"""
version = self.get_version_str()
if not None in [self.tc_name, version] and self.operator == self.OPERATOR_MAP['==']:
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the idea of the first part of the if? get_version_str can't be None. maybe you just meant if self (see the is_valid and __bool)

Copy link
Member Author

Choose a reason for hiding this comment

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

no, I don't need if self

you're right that checking the version (and self.tc_name) is stupid; if no version was specified (e.g. with [goolf]), the version will be set to 0.0.0 anyway...

extended unit tests to cover as_dict() too

@boegel
Copy link
Member Author

boegel commented Jan 30, 2014

@stdweird: covered your remarks, had questions w.r.t. a couple of them

A unit test for the (basic) dependencies support is now present as well, after I added the minor missing pieces...

@boegel
Copy link
Member Author

boegel commented Jan 30, 2014

@stdweird: also fixed the self.default and self.supported remarks, after it became clear what you were pointing out

@stdweird
Copy link
Contributor

@boegel looks ok to me

@boegel
Copy link
Member Author

boegel commented Jan 30, 2014

@stdweird: maintainer is not enforced for now:

(from easybuild/framework/easyconfig/format/two.py)

    AUTHOR_REQUIRED = True
    MAINTAINER_REQUIRED = False

Merging in, thanks for the review!

boegel added a commit that referenced this pull request Jan 30, 2014
[formatv2] split off supported section from default section, add preliminary support for a dependencies section
@boegel boegel merged commit 082e8be into easybuilders:develop Jan 30, 2014
@boegel boegel deleted the formatv2_supported_deps branch January 30, 2014 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants