feat: add atlas pull --expand-glob to expand bash glob file patterns
#45
Conversation
|
Thanks for the pull request, @OmarIthawi! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
openedx-webhooks
added
the
open-source-contribution
| Dismissed alternatives | ||
| ********************** | ||
|
|
||
| 1. Support links again: | ||
| ----------------------- | ||
|
|
||
| Previous tests on unknown git version showed to support links in April 2023, | ||
| but it had no automated tests and broke somewhere during the | ||
| `git v2.25.1 version support`_ we've added. | ||
|
|
||
| The sparse-checkout part is already complex, so we'll not touch it. | ||
|
|
||
| 2. Make complete copies of the translations in `edx-platform-links`_ | ||
| --------------------------------------------------------------------- | ||
| This is the most viable and simple alternative to glob patterns. | ||
|
|
||
| If it turns out the ``--expand-glob`` option is flaky to work with, we'll | ||
| fallback to this. | ||
|
|
||
| This _may_ complicate the GitHub Transifex App sync process which we prefer | ||
| to keep simple and avoid adding additional step to ensure the | ||
| `edx-platform-links`_ have full copies of the XBlock and plugins translations | ||
| instead of symbolic links. | ||
|
|
||
| 3. Use the `edx-platform-links`_ to store the original files | ||
| ------------------------------------------------------------ | ||
|
|
||
| This is a viable option as well but needs to refactor the Transifex | ||
| configuration which could reset the translations. | ||
|
|
||
| Therefore, we'll only fallback to this if the glob patterns has problems. |
There was a problem hiding this comment.
It's not fully clear to me how these alternatives would accomplish what is being done with --expand-glob
It seems each of the alternatives has a great description of why we decided against it, but not a full explanation of how it would work if we decided to go that route.
I also think the lines about which alternative we'll fall back to if the glob expansion is problematic can be removed. We can reference the alternatives we rejected and decide on a new path forward if that time comes.
There was a problem hiding this comment.
Good points. Both are addressed.
I'll merge once tests passes.
Example: 'atlas pull translations/*/done' pulls 'atlas pull translations/DoneXBlock/done' if it exists.
|
@OmarIthawi 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
|
🎉 This PR is included in version 0.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Expand glob pattern in
directory_fromwhich is useful for instance to pull XBlocks in which the directory name isn't known but the module name is.Check the attached
0001-support-glob-pattern.rstdecision document for more context.This contribution is part of the FC-0012 project which is sparked by the Translation Infrastructure update OEP-58.
Example
atlas pull -g translations/*/donepullsatlas pull translations/DoneXBlock/doneif it exists.TODO
edx-platform-linksfrom translations repoatlas pullfor the edx-platform and its plugins FC-0012 edx-platform#33166