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

Logical operators #82

Merged
merged 27 commits into from
Aug 15, 2021
Merged

Logical operators #82

merged 27 commits into from
Aug 15, 2021

Conversation

vahmelk99
Copy link
Contributor

Please check my translation.

@CLAassistant
Copy link

CLAassistant commented Aug 9, 2021

CLA assistant check
All committers have signed the CLA.

@bugron
Copy link
Contributor

bugron commented Aug 9, 2021

Thanks for the PR @vahmelk99. I'll be reviewing it soon.

Copy link
Contributor

@bugron bugron left a comment

Choose a reason for hiding this comment

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

Please do not be discouraged by the number of comments. It is normal for this project. Once the suggested changes are implemented I'll give it another round of reviews. Thanks.

1-js/02-first-steps/11-logical-operators/article.md Outdated Show resolved Hide resolved
1-js/02-first-steps/11-logical-operators/article.md Outdated Show resolved Hide resolved
1-js/02-first-steps/11-logical-operators/article.md Outdated Show resolved Hide resolved
1-js/02-first-steps/11-logical-operators/article.md Outdated Show resolved Hide resolved
1-js/02-first-steps/11-logical-operators/article.md Outdated Show resolved Hide resolved
@javascript-translate-bot

Please make the requested changes. After it, add a comment "/done".
Then I'll ask for a new review 👻

@bugron
Copy link
Contributor

bugron commented Aug 10, 2021

@vahmelk99 also for future PRs the name of the PR should exactly match the title of the article you are translating (see #1). For example the title of this PR should have been Logical operators.

@vahmelk99
Copy link
Contributor Author

@vahmelk99 also for future PRs the name of the PR should exactly match the title of the article you are translating (see #1). For example the title of this PR should have been Logical operators.

Should I create a new PR?

@bugron
Copy link
Contributor

bugron commented Aug 11, 2021

@vahmelk99 nope, just have that noted for future PRs. Thanks.

Copy link
Contributor

@bugron bugron left a comment

Choose a reason for hiding this comment

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

@vahmelk99 please make sure there are no unnecessary line shifts here and there, also make sure to restore original formatting and new lines where mentioned. Thanks for you contribution.

@javascript-translate-bot

Please make the requested changes. After it, add a comment "/done".
Then I'll ask for a new review 👻

@vahmelk99
Copy link
Contributor Author

@bugron thanks for your time.

@vahmelk99
Copy link
Contributor Author

/done

@bugron
Copy link
Contributor

bugron commented Aug 13, 2021

@vahmelk99 thanks for your efforts but unfortunately not all the comments are addressed. GitHub might hide some of them so please make sure everything is in order before we can move on to merging this PR. Thanks.

Copy link
Contributor

@bugron bugron left a comment

Choose a reason for hiding this comment

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

@vahmelk99 looks superb! Thanks again for your contribution! I've fixed a few line shifts and svg translation stuff and we're ready to merge this PR.

@bugron bugron merged commit a99cafd into javascript-tutorial:master Aug 15, 2021
@bugron bugron changed the title 11-logical-operators translated Logical operators Aug 15, 2021
@vahmelk99
Copy link
Contributor Author

@vahmelk99 looks superb! Thanks again for your contribution! I've fixed a few line shifts and svg translation stuff and we're ready to merge this PR.

In the SVG file, I fixed the coordinates of the text, so there were not any overflowed texts.

@bugron
Copy link
Contributor

bugron commented Aug 15, 2021

@vahmelk99 the thing is that SVG file should not be modified directly, there is a note about that in README.md. A special images.yml file is used to define SVG text translations and text positions. But its a bit tricky to see the results here because you'll also need to set up the server, run a gulp job to generate new images based on that yml file and start the project (local version of javascriptinfo.com) to check whether everything looks as intended or not. Cheers.

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

Successfully merging this pull request may close these issues.

4 participants