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

Generating class in other module. #236

Merged
merged 1 commit into from Mar 14, 2018
Merged

Conversation

dryobates
Copy link
Contributor

It adds ability to generate class in module other than current.

@@ -42,15 +42,16 @@ def create_package(project, name, sourcefolder=None):

class _Generate(object):

def __init__(self, project, resource, offset):
def __init__(self, project, resource, offset, goal_resource=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be invalid for GenerateModule or GeneratePackage, there's nothing additional to do for these. However, being as they override get_changes, maybe this doesn't matter since it would just be ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I admit that it is poor style to declare fields in parent class that are not used in child class. I have chosen it because it needs the smallest changes.

As an alternative I could change hierarchy so that it would look something like:

  • _ModuleElementGenerate(_Generate)
  • GenerateClass(_ModuleElementGenerate)
  • GenerateFunction(_ModuleElementGenerate)
  • GenerateVariable(_ModuleElementGenerate)
  • GenerateModule(_Generate)
  • GeneratePackage(_Generate)

Should I change it?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I agree it's better as-is, and won't cause any harm. Thanks!

@soupytwist soupytwist self-assigned this Mar 14, 2018
@soupytwist soupytwist merged commit 0d79ddd into python-rope:master Mar 14, 2018
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.

None yet

2 participants