Skip to content

Empty line no longer added to start of strings files on sort#180

Merged
Jeehut merged 2 commits intoFlineDev:mainfrom
hactar:main
Apr 20, 2020
Merged

Empty line no longer added to start of strings files on sort#180
Jeehut merged 2 commits intoFlineDev:mainfrom
hactar:main

Conversation

@hactar
Copy link
Copy Markdown
Contributor

@hactar hactar commented Apr 19, 2020

As discussed in #178 - when normalising and sortByKeys = true, BartyCrouch adds a new line to the start of every strings file. This PR resolves this issue.

Copy link
Copy Markdown
Member

@Jeehut Jeehut 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, but please improve the Changelog entry, then I'll merge, thank you for this. 💯
Funny that there was an explicit \n there, but the easier the fix. :)

CHANGELOG.md Outdated
- None.
### Fixed
- None.
- Normalize sortByKeys no longer adds empty line to begining of .strings file.
Copy link
Copy Markdown
Member

@Jeehut Jeehut Apr 20, 2020

Choose a reason for hiding this comment

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

Two whitespaces missing at the end of this line.

CHANGELOG.md Outdated
### Fixed
- None.
- Normalize sortByKeys no longer adds empty line to begining of .strings file.
Issues: [#178](https://github.com/Flinesoft/BartyCrouch/issues/178)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two whitespaces missing at the beginning of this line. Also, please singularize to Issue, add the PR: link and yourself as the Author:.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added the changes. Please note that requiring the PR number in the change log creates a chicken and egg problem as I don't know the PR number until I actually try to create the PR, at which point in time I've already committed and pushed ;)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The idea is that you start a PR with your changes and make a commit like Document changes of PR #180 after the PR is online with the changes to the Changelog. Maybe I should document this somewhere. But in general you are right and I don't require the PR, I just mentioned it because the Issue and Author also were not quite right. :)

@hactar hactar requested a review from Jeehut April 20, 2020 07:46
@Jeehut Jeehut changed the title Empty line no longer added to start of strings files on sort. Empty line no longer added to start of strings files on sort Apr 20, 2020
@Jeehut
Copy link
Copy Markdown
Member

Jeehut commented Apr 20, 2020

@hactar Noticed one more thing: You've added a . at the end of your commit message. Would you be up to fix that quickly by doing a rebase? If not, I can also merge as is, it's not a big deal.

@hactar
Copy link
Copy Markdown
Contributor Author

hactar commented Apr 20, 2020

Sorry bout that, but I'm afraid that goes beyond the scope of things I can quickly do via git.

@Jeehut
Copy link
Copy Markdown
Member

Jeehut commented Apr 20, 2020

Ok, sure, no worries. Merging, thank you! 🎉

@Jeehut Jeehut merged commit a1159aa into FlineDev:main Apr 20, 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.

2 participants