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

Move Python files over from activist repo #1

Closed
andrewtavis opened this issue Dec 1, 2024 · 16 comments
Closed

Move Python files over from activist repo #1

andrewtavis opened this issue Dec 1, 2024 · 16 comments
Assignees
Labels
help wanted Extra attention is needed localization Add or correct localizations

Comments

@andrewtavis
Copy link
Member

This issue would be to migrate the files from activist-org/activist/frontend/i18n/check to this repo to an appropriate location so that they're accessible by the GitHub action.

We'd need to research how to do this appropriately such that the repo structure is standard for how other GitHub actions are structured.

@andrewtavis andrewtavis added the help wanted Extra attention is needed label Dec 1, 2024
@OmarAI2003
Copy link
Contributor

Hey!
This one’s for me, right? 😄
Sorry for the delay—I had a lot going on that kept me from diving into it!

@andrewtavis
Copy link
Member Author

Is for you, yes :) Thanks for your interest in helping out here, @OmarAI2003!

@andrewtavis
Copy link
Member Author

I'm really not sure how to structure a GitHub action 🤔 Do you want to do some research and then let me know how we'd best structure this? Went from a template when I set it all up, but maybe there are some things that need to be changed. Your suggestions on the Python files would also of course be very welcome!

@OmarAI2003
Copy link
Contributor

I’ll take time to research this from scratch and try to come up with a good solution. Sorry for starting on this a bit late!

@andrewtavis
Copy link
Member Author

No need to apologize whatsoever! Happy to be able to work with you again! 😊

@andrewtavis
Copy link
Member Author

andrewtavis commented Dec 18, 2024

Another thing that would be good to get your feedback on is how to do the full check lifecycle here as well 🤔 Ideally we'd be able to run the checks locally as well, so are we talking a Python package that does the checks, as this couldn't be done from the action? Can that be in the same repo, or should we split the check functionality out into a second repo and have this one import it and just be the action?

Let me know what your thoughts are :)

@andrewtavis
Copy link
Member Author

The Python package could be something like a CLI that reads in from a configuration YAML file that's generated on first run, and the YAML file would similarly be made for the action. We could also simply do the package and have it run in an action as well.

@OmarAI2003
Copy link
Contributor

Is it necessary to migrate the checks to this repo, or would it be fine to keep them in the current activist repo since they seem closely related to the frontend? We could set up the workflow there as well. I’m not fully aware of the bigger scope, so I’m unsure which option would be better in the future

@andrewtavis
Copy link
Member Author

andrewtavis commented Dec 20, 2024

Yes the checks should basically load in the code from this repo. Big thing that I'm realizing though is that we need to also be able to run them locally 🤔 What would you say on this? Should we make a Python package that activist and other projects can load in and define with a config file to check their i18n files? Maybe the idea of a GitHub action is too restrictive, and to set up an i18n check on PRs we'd just pip install this into a PR workflow and then run it with the repository config file?

@andrewtavis
Copy link
Member Author

andrewtavis commented Dec 20, 2024

Big thing is that there have been comments from people saying that they think we're managing i18n really well and that these workflows would help their project, so that code needs to move out of activist and be loaded in from here.

From there we start getting into some very interesting possibilities for really refining the code here, which I think you could really help with :) For instance, a common complaint that we get with our i18n checks is that the keys have to appear completely in the code:

// Has to be like this to be picked up as being present in the code:
$t("components.component.identifier_0")
$t("components.component.identifier_1")

// Can't be like this:
for (i in [0, 1]) {
  $t(`components.component.identifier_${i}`)
}

Which inherently means that code in some cases can't be made more dry. It'd be great if we could get the current setup working, and then from there we could explore how to make the second example above work such that it'd really be able to be dropped into a wide variety of codebase!

@OmarAI2003
Copy link
Contributor

I’ve thought about this more, and I think we should create a standalone repository for the i18n checks. This would make the logic modular and reusable in different contexts. We could then use the package in the GitHub Action workflow for PR checks. For running checks locally, we could integrate it with a pre-commit hook to call the package directly. What do you think ?

@andrewtavis
Copy link
Member Author

I think that that makes sense for sure :) For now let's maybe just rename this one as we already have the issue for migrating the code over. From there it would be good to write some tests as well so we know that all's working :) Would also likely help you understand the process a bit more 😊

@andrewtavis
Copy link
Member Author

Ok project renamed :) I'll remake i18n-check-action once we have the code migrated over here :) I'll make another issue now for basic testing. Feel free to commit the current files to a new src/i18n_check/checks directory :) Then we'll work on setting up the YAML file with the settings 😊

@andrewtavis
Copy link
Member Author

Making this a CLI makes sense to you? So we have a i18n_check_config.yaml and then the user can simply run i18n-check in their terminal and this file is read in? We'd then have a command where this file would be generated based on inputs from the user, and if none is found the user is asked to generate one?

@andrewtavis
Copy link
Member Author

#2 is made for the testing. Once we agree on the CLI functionality I'll also make issues for the functionality 😊

@andrewtavis
Copy link
Member Author

Closed by #3 🚀 Let me know if you'd have interest in working on the rest of the setup, @OmarAI2003! I think that writing tests in #2 would make sense, and let's definitely discuss a bit how we'd like the CLI to work over here. You'd be welcome to set that up as well 😊

@github-project-automation github-project-automation bot moved this from Todo to Done in activist Board Dec 28, 2024
@andrewtavis andrewtavis added the localization Add or correct localizations label Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed localization Add or correct localizations
Projects
Archived in project
Development

No branches or pull requests

2 participants