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

[core] update l10n issue with a single command line #3588

Merged
merged 7 commits into from
Jan 17, 2022

Conversation

alexfauquette
Copy link
Member

No description provided.

@alexfauquette alexfauquette self-assigned this Jan 10, 2022
@alexfauquette alexfauquette added the core Infrastructure work going on behind the scenes label Jan 10, 2022
package.json Outdated Show resolved Hide resolved
@m4theushw
Copy link
Member

Do you plan to create the GitHub workflow? It would be nice to have this automated.

@alexfauquette
Copy link
Member Author

I'm not planning to do it for now, because I do not know exactly how files are managed in github workflow. Which could be a problem, especially with the following part of the code.

function git(args: any) {
  return new Promise((resolve, reject) => {
    exec(`git ${args}`, (err, stdout) => {
      if (err) {
        reject(err);
      } else {
        resolve(stdout.trim());
      }
    });
  });
}

I agree it would be better, but for now, updating the issue at least once a week during the release seems enought

@@ -63,6 +63,16 @@ You can use the following script in your browser console on any GitHub page to a
4. Release the versions on NPM: `yarn release:publish` (you need your 2FA device).
5. Create a new tag named with the release you just did `git tag v4.0.0-alpha.30 && git push upstream --tag`

### Update l10n issue
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this section. I'll open another PR to add the GitHub workflow to always keep this issue up-to-date.

Copy link
Member Author

Choose a reason for hiding this comment

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

So I let it, and you will remove it in the PR to add the GitHub workflow?

Copy link
Member

Choose a reason for hiding this comment

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

You can keep it. Once we have this running automatically I don't think it's necessary to mention it in the README, it's one thing less to care about.

@m4theushw
Copy link
Member

I'm not planning to do it for now, because I do not know exactly how files are managed in github workflow. Which could be a problem, especially with the following part of the code.

It works in the same way as in your machine. We first git checkout with a special GitHub action, then yarn install and finally run the command to update the issue.

https://github.com/mui-org/material-ui/blob/master/.github/workflows/ci.yml#L22

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 14, 2022
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 14, 2022
@mui-bot
Copy link

mui-bot commented Jan 14, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 206.7 349.5 311 282.88 62.822
Sort 100k rows ms 464.6 776.1 641.7 622.58 101.892
Select 100k rows ms 167.2 327.9 206.7 227.02 54.957
Deselect 100k rows ms 106.5 272.6 181.1 184.22 61.094

Generated by 🚫 dangerJS against 36035fa

@alexfauquette
Copy link
Member Author

@m4theushw I made the script to make the update when there is a push on master. If you can verify it. I don't know how to test it except merging and hope it works

@@ -0,0 +1,29 @@
name: CI
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name: CI
name: Update missing translations

# Don't need playwright in this job
PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: 1
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: yarn release:tag
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: yarn release:tag
- name: yarn l10n --report

- name: yarn release:tag
run: |
git remote -v
yarn l10n --report
Copy link
Member

Choose a reason for hiding this comment

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

The GITHUB_TOKEN envvar is missing.

@m4theushw
Copy link
Member

I made the script to make the update when there is a push on master. If you can verify it. I don't know how to test it except merging and hope it works

It's a shot in the dark, we test after merge. You could configure GitHub Actions in your fork. It might not work initially but we can fix.

@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 17, 2022
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 17, 2022
@alexfauquette alexfauquette merged commit 63f3543 into mui:master Jan 17, 2022
@alexfauquette alexfauquette deleted the l10n-script branch January 17, 2022 10:53
@alexfauquette
Copy link
Member Author

Bot is live 🤖🎉

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants