Skip to content

/translators

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

[ISTC.js]: Replace (deprecated) for each loops #1203

Merged
merged 1 commit into from Dec 29, 2016

Conversation

Reviewers

@zuphilip zuphilip

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@adomasven @zuphilip @adam3smith
@adomasven
Copy link
Member

adomasven commented Dec 28, 2016

Also change inconsistent line endings to be in line with the rest of translators using \n
@zuphilip

zuphilip reviewed Dec 28, 2016
View changes

ISTC.js Outdated
@@ -12,127 +12,128 @@
"lastUpdated": "2016-11-01 14:02:29"

This comment has been minimized.

Sign in to view
Copy link
@zuphilip

zuphilip Dec 28, 2016

Collaborator

You have to increase the date here, in order to trigger the distribution to the clients.
@zuphilip

This comment has been minimized.

Sign in to view
Copy link
Collaborator

zuphilip commented Dec 28, 2016

It looks that you changed also some whitespaces because the diff shows that everything changed, where you only changed a few lines: https://github.com/zotero/translators/pull/1203/files?w=0
@adomasven

This comment has been minimized.

Sign in to view
Copy link
Member Author

adomasven commented Dec 28, 2016 •
edited

It looks that you changed also some whitespaces because the diff shows that everything changed

Yes. This is my IDE automatically converting newline characters. I scanned all translator files and it seems that this is the only one which had windows style newlines \r\n mixed in with \n. Needless to say, having mixed newlines can cause all kinds of unexpected issues, so all of those were replaced with \n, per my commit comment.

(lastUpdated field updated and force pushed)
@adomasven
[ISTC.js]: Replace (deprecated) for each loops 
Also change inconsistent line endings to be in line with the rest of
translators using `\n`
5416dd2

@adomasven adomasven force-pushed the adomasven:fix/remove-deprecated-foreach branch from 37c7895 to 5416dd2 Dec 28, 2016

@zuphilip

This comment has been minimized.

Sign in to view
Copy link
Collaborator

zuphilip commented Dec 28, 2016

Okay, that looks good for me. 👍 Changing \r\n to \n is reasonable and consistent, then. (On Windows the default for git is to allow such line endings but to check them into the Linux way.)

@adam3smith adam3smith merged commit 2b6221e into zotero:master Dec 29, 2016

@adam3smith

This comment has been minimized.

Sign in to view
Copy link
Collaborator

adam3smith commented Dec 29, 2016

Great, thanks!

@adam3smith adam3smith referenced this pull request Dec 29, 2016

Closed

Add travis check (WIP) #1156

@adomasven adomasven deleted the adomasven:fix/remove-deprecated-foreach branch Jul 19, 2017

zuphilip added a commit to zuphilip/translators that referenced this pull request Mar 28, 2018

@adomasven @zuphilip
[ISTC.js]: Replace (deprecated) for each loops (zotero#1203) 
Also change inconsistent line endings to be in line with the rest of
translators using `\n`
bfd80ff

zuphilip added a commit to zuphilip/translators that referenced this pull request Mar 28, 2018

@adomasven @zuphilip
[ISTC.js]: Replace (deprecated) for each loops (zotero#1203) 
Also change inconsistent line endings to be in line with the rest of
translators using `\n`
bc10028
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.