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

86 encoding scheme #106

Merged
merged 10 commits into from
Aug 18, 2012
Merged

Conversation

fgeorgatos
Copy link
Collaborator

Hi,

see issue #86 in relation to this pull request.

sorry for the single pull request for all 6 commits; I still have to learn some git ropes to refactor the commits...

I will be carefully checking the remarks on github though... be strict to make the first shot just right.

cheers,
Fotis

fgeorgatos and others added 6 commits August 18, 2012 14:22
Signed-off-by: Fotis Georgatos <fotis.georgatos@uni.lu>
…ng(name)

Signed-off-by: Fotis Georgatos <fotis.georgatos@uni.lu>
…), name))

Signed-off-by: Fotis Georgatos <fotis.georgatos@uni.lu>
correct naming of classes for Python packages nose, numpy and scipy
from easybuild.tools.module_generator import ModuleGenerator
from easybuild.tools.modules import Modules, get_software_root
from easybuild.tools.toolkit import Toolkit
from easybuild.tools.systemtools import get_core_count


EBCLASSPREFIX = 'eb_'
Copy link
Member

Choose a reason for hiding this comment

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

With the prefixing, and certainly with the encoding of class names, wer'e violating (http://www.python.org/dev/peps/pep-0008/#class-names)[the PEP008 class naming rule].
But, since we have a good technical reason for that, that's OK (we just need to document our motivation).

However, I think I prefer the change the prefix to EB_ (I don't think we discussed upper vs lowercase prefixes?).

Having class names start with a capital letter makes more sense to me.

Your thoughts?

Signed-off-by: Fotis Georgatos <fotis.georgatos@uni.lu>
Signed-off-by: Fotis Georgatos <fotis.georgatos@uni.lu>
Signed-off-by: Fotis Georgatos <fotis.georgatos@uni.lu>

def encode_string(name):
"""

Copy link
Member

Choose a reason for hiding this comment

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

get rid of this empty line, it might screw up the generated documentation

Signed-off-by: Fotis Georgatos <fotis.georgatos@uni.lu>
@boegel
Copy link
Member

boegel commented Aug 18, 2012

Thanks for fixing all my remarks, and congratulations on your first contribution to EasyBuild!

boegel added a commit that referenced this pull request Aug 18, 2012
@boegel boegel merged commit 80c2497 into easybuilders:develop Aug 18, 2012
@fgeorgatos
Copy link
Collaborator Author

This was referenced Aug 25, 2012
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