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

[IMP] Add pylint check missing newline in extra extension files. #22

Conversation

luiseevaquer
Copy link

[IMP] Add pylint check trailing newlines in other extension files.

@luiseevaquer
Copy link
Author

@moylop260 @pedrobaeza @max3903
Please your feedback

@pedrobaeza
Copy link
Member

I see correct this trailing line (it helps in the diff if you add a new line at the end). Even more, in .po files, Odoo or Poedit adds at least 2 trailing lines at the end.

@luiseevaquer luiseevaquer force-pushed the master-oca-traling-newlines-otherfiles-dev-lescobarvx branch from b2861b0 to 872e7bb Compare April 19, 2016 18:18
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 872e7bb on vauxoo-dev:master-oca-traling-newlines-otherfiles-dev-lescobarvx into 8bfeace on OCA:master.

@moylop260
Copy link
Collaborator

@lescobarvx
@pedrobaeza is right!

We need change this check for cases where don't exists a newline at final file.
Similar to the red that show github... (Let me show you a example of this)

@moylop260
Copy link
Collaborator

moylop260 commented Apr 20, 2016

@lescobarvx
I'm talking about follow one:
without newline

@luiseevaquer luiseevaquer force-pushed the master-oca-traling-newlines-otherfiles-dev-lescobarvx branch from 872e7bb to df28ec3 Compare April 20, 2016 20:54
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling df28ec3 on vauxoo-dev:master-oca-traling-newlines-otherfiles-dev-lescobarvx into 8bfeace on OCA:master.

if os.stat(ext_file).st_size > 0:
fp.seek(-2, os.SEEK_END)
last_line = fp.readline()
if last_line[-1:] not in ['\n', '\r']:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use last_line.endswith('\n') or last_line.endswith('\r')

@luiseevaquer luiseevaquer force-pushed the master-oca-traling-newlines-otherfiles-dev-lescobarvx branch from df28ec3 to 7c45287 Compare April 20, 2016 21:40
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 7c45287 on vauxoo-dev:master-oca-traling-newlines-otherfiles-dev-lescobarvx into 8bfeace on OCA:master.

@luiseevaquer luiseevaquer force-pushed the master-oca-traling-newlines-otherfiles-dev-lescobarvx branch from 7c45287 to 11f9cfe Compare April 20, 2016 21:50
@luiseevaquer luiseevaquer changed the title [IMP] Add pylint check trailing newlines in other extension files. [IMP] Add pylint check missing newline in extra extension files. Apr 20, 2016
@luiseevaquer luiseevaquer force-pushed the master-oca-traling-newlines-otherfiles-dev-lescobarvx branch 2 times, most recently from 4171029 to 47c25e8 Compare April 20, 2016 22:01
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 47c25e8 on vauxoo-dev:master-oca-traling-newlines-otherfiles-dev-lescobarvx into 8bfeace on OCA:master.

@moylop260
Copy link
Collaborator

👍

@moylop260 moylop260 force-pushed the master-oca-traling-newlines-otherfiles-dev-lescobarvx branch from 47c25e8 to c49705e Compare May 9, 2016 05:14
@moylop260
Copy link
Collaborator

Rebased

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling c49705e on vauxoo-dev:master-oca-traling-newlines-otherfiles-dev-lescobarvx into 6d244c6 on OCA:master.

@nhomar
Copy link
Member

nhomar commented May 11, 2016

👍

@pedrobaeza
Copy link
Member

Please rebase

@moylop260 moylop260 force-pushed the master-oca-traling-newlines-otherfiles-dev-lescobarvx branch from c49705e to 0af845b Compare May 11, 2016 11:16
@moylop260
Copy link
Collaborator

rebased

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 0af845b on vauxoo-dev:master-oca-traling-newlines-otherfiles-dev-lescobarvx into bd1e9f4 on OCA:master.

@pedrobaeza
Copy link
Member

👍

@pedrobaeza pedrobaeza merged commit 7148966 into OCA:master May 11, 2016
@moylop260
Copy link
Collaborator

Thanks @pedrobaeza

@luisg123v luisg123v deleted the master-oca-traling-newlines-otherfiles-dev-lescobarvx branch August 6, 2020 02:30
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

6 participants