comprehensive-rust icon indicating copy to clipboard operation
comprehensive-rust copied to clipboard

ja: tooling check

Open CoinEZ-JPN opened this issue 1 year ago • 10 comments

ja: tooling check (google#652)

CoinEZ-JPN avatar May 23 '23 07:05 CoinEZ-JPN

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar May 23 '23 07:05 google-cla[bot]

Thanks a lot for putting this up @CoinEZ-JPN!

I've invited a few people at Google to help out with the translation as well. I hope you can all coordinate on #652.

mgeisler avatar May 23 '23 09:05 mgeisler

Hi, thank you for starting Japanese translation!

It seems that your commit contain two changes: (1) auto-generated po/ja.po file and (2) your own translations. Would you mind splitting them into different pull requests? Otherwise, the diff is so big that i't's hard to put inline review comments on GitHub UI. Thanks!

keiichiw avatar May 23 '23 10:05 keiichiw

@keiichiw I'll be creating a new PR, given how cherry-picking doesn't seem to be a reasonable option the way I committed. In any case, just a quick overview on the approach (commit/PR breakdowns) I intend to take after having had a closer look at how the ja.po is configured.

  • Table of Contents headers
  • Intro to course (Ch. 1-2)
  • Day 1: Morning (3-7)
  • Day 1: Afternoon
  • ...
  • Day 3: Afternoon
  • Android (31-37) and so on. Will leave proper comments on the PR to make it easier.

As I'm not familiar with .po or git at this point, please let me know if you'd like a dif breakdown or granularity.

CoinEZ-JPN avatar May 24 '23 22:05 CoinEZ-JPN

Okay, I understand. I just uploaded a pull request to add an unmodified ja.po file at https://github.com/google/comprehensive-rust/pull/680. So, once it's merged, could you please pull it and rebase this branch on it?

The changes you listed in the comment looks reasonable to have them in this PR, I think. So, you'll no longer need to create a new PR. Sorry for the confusion.

I think the git commands are like the followings:

$ git checkout <your branch>
$ git remote add upstream [email protected]:google/comprehensive-rust.git
$ git fetch upstream
$ git rebase upstream/main

Then you can push the change to the same branch again.

Thanks in advance!

keiichiw avatar May 25 '23 02:05 keiichiw

Apol for the tardiness. Followed instructions and committed - just the table of contents.

momotaro1105 avatar May 25 '23 07:05 momotaro1105

As I'm not familiar with .po or git at this point, please let me know if you'd like a dif breakdown or granularity.

Hey @CoinEZ-JPN, I just looked at the diff of this PR and it looks good from a pure Git point of view — I'm afraid I cannot evaluate the correctness of the translation itself, I can only admire the beautiful script :smile:

mgeisler avatar May 25 '23 08:05 mgeisler

@CoinEZ-JPN and others, please see #683 for a discussion about where to put a translation dictionary.

mgeisler avatar May 25 '23 08:05 mgeisler

@CoinEZ-JPN Thank you for updating your patch! It seems that the branch you pushed was based on a bit outdated branch so the diff was still odd. (For example, it didn't contain my recent commit) So, I updated the branch and cherry-picked your latest commit. Sorry for modifying your branch directly, but I think it becomes clean now.

A few tips for your future work on GitHub:

  • It'd be nice to create a branch when you open a pull request instead of using main.
  • if you want to update your branch for pull request, I think git rebase is usually better. The workflow is like this:
$ git checkout main 
$ git pull upstream main # main branch should be synced with the upstream's main
$ git checkout your-branch-for-pull-request
$ git rebase main
$ git push origin your-branch-for-pull-request

Then, you can keep your branch clean and up-to-date.

Anyway, I think the branch is now ready for review. Let me take a look at your patch.

keiichiw avatar May 25 '23 08:05 keiichiw

thank you @keiichiw last thing I want is for you to walk me through this, but such is the case for now

will continue with the improvements

CoinEZ-JPN avatar May 25 '23 12:05 CoinEZ-JPN

And, I just renamed the pull request title to clarify what is translated in this PR

keiichiw avatar May 29 '23 04:05 keiichiw