Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ISBNtoBibTeX error mesage is now more clear #689

Closed
wants to merge 4 commits into from
Closed

ISBNtoBibTeX error mesage is now more clear #689

wants to merge 4 commits into from

Conversation

Siedlerchr
Copy link
Member

This is my first Pull Request.

Refs #684.

@koppor
Copy link
Member

koppor commented Jan 22, 2016

Your commit 33bd952 changes each line in the file. Did you use Eclipse eGit? This causes issues at Windows because of the line endings or to change to LF at the git checkout. I would recommend to always use git gui instead of eGit.

Please read CONTRIBUTING.md which explains some details on the PRs. Especially that one should not use the master branch: All future updates to your master branch will be shown here in this PR. This is a known issue.

Finally, for small changes such as merging the master branch or the correct usage of the localization, please do

  1. git fetch upstream --prune
  2. git merge upstream/master
  3. git reset upstream/master
  4. git gui

In git gui, review and commit your changes. Then do a force push to overwrite your changes here. This squashes all commits into a single one. If you want to keep multiple commits, please continue reading at using-git-rebase.

The first two steps are basically https://help.github.com/articles/syncing-a-fork/, where step 4 is skipped.

Hint: At Windows: set the environment variable LANG to en.

You can also send me a private mail if you have any issues in using git.

@koppor koppor changed the title Error mesage is now more clear #684 ISBNtoBibTeX error mesage is now more clear Jan 22, 2016
@simonharrer
Copy link
Contributor

And can you please merge your 4 commits into 1 commit as well as add a changelog entry describing your change?

What is more:

  • Localization.lang does not use %s as placeholders but %0 and %1 etc.
  • When updating Localization.lang keys, we have a test in place which ensures that the localizations are correct. You may need to look into this as well.

@Siedlerchr
Copy link
Member Author

Thanks for all the tips. I will look at this today in the evening.

@simonharrer
Copy link
Contributor

@Siedlerchr I think this would be helpful to be merged in for the next release. Could you perhaps complete the PR in the next days?

@Siedlerchr
Copy link
Member Author

@simonharrer I've been busy, but I will try to finish the PR tomorrow.

@simonharrer
Copy link
Contributor

Duplicate of #761

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants