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

Improve PEP 8 conformance of generated code. #106

Closed

Conversation

techdragon
Copy link
Contributor

  • Removed blank line that was introducing too many spaces between classes in the operations block.

- Removed blank line that was introducing too many spaces between classes in the operations block.
@ngnpope
Copy link
Member

ngnpope commented May 9, 2017

We'll have to think about this one. I spent quite a while trying to get this right, but I think that it is somewhat impossible while soapfish uses templates to generate code. One change to fix white space seems to unfix another attempt to resolve the problem code generated by a different WSDL.

@techdragon
Copy link
Contributor Author

@pope1ni I agree that the code generation is tricky, I'm still formatting up a larger change to the generation template to fix #97 and part of getting that as concise as possible made it seem like a good idea to separate out any 'other' fixes into their own changes.

Since the test cases did pass, I'm curious if there is any hesitation about this particular line of whitespace, or wether its an overall (and understandable) need for caution when making changes to the code generation templates.

@ngnpope
Copy link
Member

ngnpope commented May 10, 2017

I think that I have actually already dealt with this one recently, but by removing a different line:

4744fa3#diff-636588725eabc5a0e8aee1288db90ad6L27

I wouldn't particularly trust our test cases at the moment - a lot of work needs to be done here. I think that some tests are obsolete, some are duplicates, some are lacking proper/enough assertions and coverage is only ~88%.

Please close this if you think that my existing change resolves this. Thanks.

@techdragon
Copy link
Contributor Author

I'm closing it for now since its so small and theres much work to be done on more robust code generation, worrying about a few newlines in the old code, when I'm trying to write a new code generator feels rather silly.

@techdragon techdragon closed this May 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants