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

Trivial: Remove duplicate EOF trailing newlines #13330

Merged
merged 3 commits into from
Nov 2, 2019

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Oct 31, 2019

Describe problem solved by this pull request
When merging master into a branch the and experiencing conflicts the merge can be completed after resolving the conflicts using git commit but with the currently existing EOF trailing newlines you get a bunch of messages like path/filename:linenumber: new blank line at EOF and it doesn't continue.

Describe your solution
Since these newlines are useless anyways I suggest to just remove them.

Test data / coverage
It's a trivial change and still compiles.

Additional context
I'm still following up to create usefull prs out of #12072 and using the oportunity to learn merging correctly since I have to still keep it up to date to take over missing things.

because they can screw up git when merging branches.
bkueng
bkueng previously approved these changes Oct 31, 2019
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Can you remore https://github.com/PX4/Firmware/blob/master/Tools/astyle/pre-commit#L50? I don't think it's required, since we have astyle, and it is the cause of such issues.

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Oct 31, 2019

I don't think it's required, since we have astyle, and it is the cause of such issues.

But astyle doesn't get applied to CMake, Python, ... files and then we would suddenly have trailing whitespaces everywhere again not?

EDIT: But it should write why it fails.

@bkueng
Copy link
Member

bkueng commented Oct 31, 2019

But astyle doesn't get applied to CMake, Python, ... files and then we would suddenly have trailing whitespaces everywhere again not?

Yeah that's a good point. But we would have that problem already since not everyone uses the pre-commit hook and it does not run on CI either. So I don't think it would make things worse.
If we want it, we should add it to CI as well.
Either way works for me.

dagar
dagar previously approved these changes Oct 31, 2019
@dagar dagar merged commit cc06009 into master Nov 2, 2019
@dagar dagar deleted the remove-duplicate-eof-newlines branch November 2, 2019 13:49
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.

4 participants