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

V1 for Automation Website translations Issues Generation Ref: Issue 325 #407

Open
wants to merge 64 commits into
base: main
Choose a base branch
from

Conversation

sergioarmgpl
Copy link
Contributor

Hi Dear TAG Env Team, this is the final the work that we are doing with @Dianmz.

This includes a workflow called Check outdated content, that generates new Issues for Languages for ES & ZH languages at the moment. Currently the issues are assigned to Diana and I, but should be modified to the new groups that you suggested or new ones.

The issues will look like this:
Screenshot 2024-04-30 at 22 05 41

And inside the issue will look like this:
Screenshot 2024-04-30 at 22 05 53

cc: @leonardpahlke @guidemetothemoon

sergioarmgpl and others added 30 commits April 3, 2024 22:45
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Dianmz <dcmc2297@gmail.com>
Signed-off-by: Dianmz <dcmc2297@gmail.com>
Signed-off-by: Dianmz <dcmc2297@gmail.com>
Signed-off-by: Dianmz <dcmc2297@gmail.com>
Signed-off-by: Dianmz <dcmc2297@gmail.com>
Signed-off-by: Dianmz <dcmc2297@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>

feat: added branches main to check outdated content workflow
Signed-off-by: leonardpahlke <leonard.pahlke@googlemail.com>
Signed-off-by: leonardpahlke <leonard.pahlke@googlemail.com>
Also includes translation of search.md file

Signed-off-by: thelooter <evekolb2204@gmail.com>
Copy link
Member

@leonardpahlke leonardpahlke left a comment

Choose a reason for hiding this comment

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

Thank you so much! I added a couple of comments.

cc @nate-double-u you may be interested in this work too. It about automatically detecting if translations to the TAG website need to happen & opens issues for translators to do translations.

.github/workflows/check-outdated-content.yml Show resolved Hide resolved
.github/workflows/check-outdated-content.yml Outdated Show resolved Hide resolved
.github/workflows/check-outdated-content.yml Show resolved Hide resolved
.github/workflows/check-outdated-content.yml Show resolved Hide resolved
.github/workflows/check-outdated-content.yml Outdated Show resolved Hide resolved
@thelooter
Copy link
Contributor

It looks like there was some commit mess-up, since there are some commits in here, that are already in master

@sergioarmgpl
Copy link
Contributor Author

sergioarmgpl commented Jun 3, 2024

Last month we talked with @guidemetothemoon about this issue, and we talked about that this work could be merged and then we can create another issues to improve this current implementation, something like the version 2 of this implementation.

Here are some comments about the suggested changes:

In this way we can tackle different issues with Diana, and separate the work.

@Dianmz cc: @leonardpahlke @guidemetothemoon

…workflow file

Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
@guidemetothemoon
Copy link
Contributor

@sergioarmgpl @Dianmz first of all, thanks for all your time and effort on this and for all the great job you've done with implementing this automation for translations! 💚

I've gone through the PR and I think that it can be a good first candidate to merge, just to kick off things and see how it works in action. Since this PR has also been active for a long time with many changes that have been added to it I think it's good to just get the initial version merged, and then work on improvements in subsequent PRs.

Based on the changes I've seen I would suggest looking into following improvements in subsequent tasks and PRs:

  • As mentioned earlier by Leo, current inline scripts are quite long and less readable so we should move them to dedicated script files that can be called by the respective GitHub Actions in the workflow. For better readability we should also consider splitting the code into smaller functions, where it's too long and does many different tasks. I believe that this will be tracked in [Action] Use a script file to automate pending translations #429
  • To ensure maintainability and possibility for other contributors to make changes in the future we need to create a dedicated documentation file in addition to having some comments inside the scripts themselves. I believe that this will be tracked in [Action] Creation of WEBSITE_DEVELOPMENT.md #430
  • To reduce duplication we should look into possibilities to loop through the list of issue templates and for every entry trigger a "Create issue" action instead of duplicating the action for a single file. I think that you can achieve something close to running an action in a loop with matrix in GH Actions. Should we create a task for this?
  • We should also look into storing supported languages (that you store in languages variable) somewhere more visible, outside of the workflow, where the list can be retrieved from, like a separate file for instance. Or even let the script check the website/content folders for languages and dynamically generate the list with languages from it. In that case we can potentially avoid relying on someone remembering to manually update this list in this script based on new languages being added. Show we create a task for this or maybe consolidate it in the same task with the previous point?
  • Should we also create a first, pilot, reviewer group for one of the languages? Maybe we should have a task for it as well so that we can create translator groups once you feel the automation is ready? So that we can avoid potentially spamming folks in the beginning, while automation is still being tested and maturing.

@leonardpahlke do you have any objections or further comments/additions regarding this?

@guidemetothemoon
Copy link
Contributor

@sergioarmgpl @Dianmz once you provide your feedback on the unresolved comments and confirm that we have tracking tasks for the improvements that you believe should be added to the automation in subsequent PRs we can merge this PR.

@sergioarmgpl
Copy link
Contributor Author

I think that the duplicated part when created issues is different when using the plugin, it overwrites if the issue already exists. But I added the last 3 points that you mentioned into the issue #429. To work on it. @guidemetothemoon @Dianmz

@sergioarmgpl
Copy link
Contributor Author

@Dianmz could you double check if I missed something

@Dianmz
Copy link
Contributor

Dianmz commented Jun 6, 2024

I have just check it one more time and I think everything it's fine.

@guidemetothemoon thanks for the feed back, there are several thing we need to improve and we're going to keep on working in them in the new issues.

cc: @sergioarmgpl

@guidemetothemoon
Copy link
Contributor

@sergioarmgpl @Dianmz sounds like we have a plan! I think that it'll be good to merge this PR and start testing this, but now I see that the "Check outdated content" build is failing - can you please check and see if we can fix it? Once the build works I can merge the PR.
Thanks!

@leonardpahlke
Copy link
Member

@sergioarmgpl @Dianmz sounds like we have a plan! I think that it'll be good to merge this PR and start testing this, but now I see that the "Check outdated content" build is failing - can you please check and see if we can fix it? Once the build works I can merge the PR.

cc @Dianmz @sergioarmgpl

@leonardpahlke
Copy link
Member

  1. We need to fix the commit history: could we squash the 61 commits
  2. "Check outdated content" build is failing

@thelooter
Copy link
Contributor

  1. We need to fix the commit history: could we squash the 61 commits

As I mentioned before, this looks like a rebase failure to me

@sergioarmgpl
Copy link
Contributor Author

Hi you all, sorry for disappear, getting update to the issue, we have to activate some permissions to fix the error we noticed the message Error: An error occurred while creating the issue. This might be caused by a malformed issue title, or a typo in the labels or assignees. Check website/content/es_files.md!
To fix this we should go to cncf/tag-env-sustainability/settings/actions and activate the following permissions:
Screenshot 2024-08-26 at 21 24 28
because the step to create the issue is falling. cc: @leonardpahlke @guidemetothemoon

Sorry for the delay.

@leonardpahlke
Copy link
Member

@sergioarmgpl / @Dianmz does one of you have time to talk about the progress of this issue in this weeks TAG Meeting? cc @guidemetothemoon @catblade

@sergioarmgpl
Copy link
Contributor Author

I think that @Dianmz will be there because I will be flying at that time

@guidemetothemoon
Copy link
Contributor

I don't have access to Actions section in the Settings of this repository to fix the permission issue - @leonardpahlke do you have access and/or can provide me access for the same? Ref. comment from Sergio: #407 (comment)

@leonardpahlke
Copy link
Member

Hi you all, sorry for disappear, getting update to the issue, we have to activate some permissions to fix the error we noticed the message Error: An error occurred while creating the issue. This might be caused by a malformed issue title, or a typo in the labels or assignees. Check website/content/es_files.md!
To fix this we should go to cncf/tag-env-sustainability/settings/actions and activate the following permissions:

updated the permissions 👍

@guidemetothemoon
Copy link
Contributor

@sergioarmgpl @Dianmz one of you may need to re-queue the build or add an option to allow for manual trigger (workflow_dispatch) to test if the worfklow works after Leo has fixed the permissions.

@sergioarmgpl
Copy link
Contributor Author

@guidemetothemoon sure I will talk with @Dianmz to change it :) cc: @leonardpahlke

Signed-off-by: Sergio Méndez <sergioarm.gpl@gmail.com>
@sergioarmgpl
Copy link
Contributor Author

I made the change for workflow_dispatch @guidemetothemoon

Copy link
Contributor

@guidemetothemoon guidemetothemoon left a comment

Choose a reason for hiding this comment

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

@sergioarmgpl @Dianmz thanks for following up on this! Minor comment from me regarding keeping the pull_request trigger option on the workflow. Other than that, it's ready to be merged!

name: Check outdated content

on:
# pull_request:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# pull_request:
pull_request:

I guess we still want to keep pull_request option so that translation automation triggers as part of PRs but we still get an option to run it manually, on-demand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed done @guidemetothemoon

@guidemetothemoon
Copy link
Contributor

@sergioarmgpl @Dianmz looks like the workflow is failing again with some rate limit - can you please check it and let me know once it's ready so that I can do the final follow-up and get it merged? Thanks! 🙌

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

Successfully merging this pull request may close these issues.

7 participants