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

extend list of valid modules classes #368

Merged
merged 8 commits into from
Dec 5, 2012

Conversation

boegel
Copy link
Member

@boegel boegel commented Nov 30, 2012

On request of @fgeorgatos, extend the list of valid module classes.

This may need to be extended in the future, but for now, it should be sufficient, and the entries that are there make sense and will/should stay there in the future (this is important, e.g. to keep compatibility with old easyconfig files).

@JensTimmerman
Copy link
Contributor

I suggest we don't hard code these checks in framework/config.py but allow them to be specified in the configuration file?

This seems to be the only long term/flexible way to do this.

@boegel
Copy link
Member Author

boegel commented Nov 30, 2012

That's right, see #209.

However, we also should have a reasonable default, and allow to use module classes like bio in the easyconfig files we ship. We can't do that without having EasyBuild know about all the classes we use.

This pull request is a first attempt to have a reasonable default, that can be easily overridden once #209 is fixed.

@JensTimmerman
Copy link
Contributor

Well, either fix bulletpoint 1 of #209 here, (the list of valid module classes is now hard coded; one should be able to extend these in easybuild_config.py )
or remove the check completely, and add it back when you fix 209.

But the hardcoded list should be removed here, not patched.

@boegel
Copy link
Member Author

boegel commented Nov 30, 2012

And then just define the default list of module classes in the default EasyBuild configuration file?

@JensTimmerman
Copy link
Contributor

Yes, people might want to change the default to fit their own liking anyway..

@boegel
Copy link
Member Author

boegel commented Nov 30, 2012

Sounds good, that's indeed a better solution.

I'll fix it, and let you know when it's ready to be reviewed.

@boegel
Copy link
Member Author

boegel commented Dec 1, 2012

Fixed @JensTimmerman's remarks, module classes can now be easily customized via the EasyBuild configuration file.
I also did some extra work on top, which is more or less related.

This pull request should now solve the following issues:

Please review, so I can merge this in.

@JensTimmerman
Copy link
Contributor

Ok

boegel added a commit that referenced this pull request Dec 5, 2012
extend list of valid modules classes
@boegel boegel merged commit ed73186 into easybuilders:develop Dec 5, 2012
Flamefire added a commit to Flamefire/easybuild-framework that referenced this pull request Apr 29, 2020
The issue was caused by self.show returning stdout and stderr
As it now returns only stderr the issue is no longer possible
Flamefire added a commit to Flamefire/easybuild-framework that referenced this pull request Apr 30, 2020
The issue was caused by self.show returning stdout and stderr
As it now returns only stderr the issue is no longer possible
Flamefire added a commit to Flamefire/easybuild-framework that referenced this pull request Apr 30, 2020
The issue was caused by self.show returning stdout and stderr
As it now returns only stderr the issue is no longer possible
Flamefire added a commit to Flamefire/easybuild-framework that referenced this pull request Apr 30, 2020
The issue was caused by self.show returning stdout and stderr
As it now returns only stderr the issue is no longer possible
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