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

[NETBEANS-4108] fixed git -> ignore should add a new line at the end … #2061

Merged
merged 2 commits into from May 22, 2020

Conversation

dmochalov
Copy link
Contributor

…of .gitignore file

Simple two line changes and added auto test for that. For more information see the issue and recommendations on SO

The GitHub native client handles the new line correctly.

try {
r = new BufferedReader(new FileReader(file));
int i;
while((i = r.read()) >-1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Space before "-".

int i;
while((i = r.read()) >-1)
{
if( i=='\n' || i=='\r')
Copy link
Contributor

Choose a reason for hiding this comment

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

Space before '=='

}
} finally {
if (r != null) {
r.close();
Copy link
Contributor

Choose a reason for hiding this comment

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

Better use try-with-resources.

Copy link
Contributor

@eirikbakke eirikbakke left a comment

Choose a reason for hiding this comment

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

Looks good!

@eirikbakke
Copy link
Contributor

Just make sure the Travis tests pass... (the failing ones probably just need to be restarted)

@dmochalov
Copy link
Contributor Author

@eirikbakke thank you for taking the time to review it. Changed the code accordingly to your comments.
By the way, can I rerun manually Travis tasks? Do I need special rights to rerun it?

@matthiasblaesing
Copy link
Contributor

Please do not squash-and-merge this - the github account seems not the provide the necessary information.

@eirikbakke
Copy link
Contributor

eirikbakke commented Apr 11, 2020

@matthiasblaesing How do I confirm, in the GitHub interface, whether the right author/email will end up in the git log upon squash-and-merge?

Also, I'm not seeing the usual "restart" button on the Travis tests. Did something change recently?

@matthiasblaesing
Copy link
Contributor

Regarding the author information: Have a look at the profile page:
On your page your full name and an Emailaddress is listed, on @dmochalov page none of that is present.

For the restart button: Looks as if either github or travis lost access tokens - I was also missing the buttons, logging out of travis and in again fixed it.

@dmochalov
Copy link
Contributor Author

Regarding the author information: Have a look at the profile page:
On your page your full name and an Emailaddress is listed, on @dmochalov page none of that is present.

@matthiasblaesing added the info on my profile page.
I thought that info in .gitconfig is enough for correct merging:

[user]
	name = Dmitry Mochalov
	email = rigthcoder@gmail.com

In git history it looks ok:
image

@matthiasblaesing
Copy link
Contributor

@dmochalov yes the author information of the commits is totally ok. The problem starts if someone chooses the "Squash and merge" button in github to merge. CLI git uses the author information from the commits to create the merged author information, github uses the information from the profile (at least from my observation) and if that is not public, the squashed commit will hold invalid information.

In this case there are three options:

  1. The commits are not squashed (discussing this is more religion, that science)
  2. You as the original author squash the commit prior to merging (after review) and the committer merges that
  3. The committer does a manual merge with CLI git

@dmochalov
Copy link
Contributor Author

@matthiasblaesing thank you for the detailed explanation.
If followed this guide to setup git hub account.
It would be nice to include the recommendations for GitHub profile settings.

@ebarboni ebarboni added this to the 12.0 milestone May 22, 2020
@ebarboni ebarboni merged commit ac4e72f into apache:master May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants