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

Add design document for multiples-url-to-the-same-repo feature #1644

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chmouel
Copy link
Member

@chmouel chmouel commented Apr 2, 2024

Start adding design document for the multiple-url-to-same-repo feature.

format of design may change in the future but this is good enough for a start i believe.

Changes

Submitter Checklist

  • 📝 Please ensure your commit message is clear and informative. For guidance on crafting effective commit messages, refer to the How to write a git commit message guide. We prefer the commit message to be included in the PR body itself rather than a link to an external website (ie: Jira ticket).

  • ♽ Before submitting a PR, run make test lint to avoid unnecessary CI processing. For an even more efficient workflow, consider installing pre-commit and running pre-commit install in the root of this repository.

  • ✨ We use linters to maintain clean and consistent code. Please ensure you've run make lint before submitting a PR. Some linters offer a --fix mode, which can be executed with the command make fix-linters (ensure markdownlint and golangci-lint tools are installed first).

  • 📖 If you're introducing a user-facing feature or changing existing behavior, please ensure it's properly documented.

  • 🧪 While 100% coverage isn't a requirement, we encourage unit tests for any code changes where possible.

  • 🎁 If feasible, please check if an end-to-end test can be added. See README for more details.

  • 🔎 If there's any flakiness in the CI tests, don't necessarily ignore it. It's better to address the issue before merging, or provide a valid reason to bypass it if fixing isn't possible (e.g., token rate limitations).


The current implementation of the Repository CR allows specifying a single URL
for the repository. This is limiting in cases where the repository is mirrored
to multiple URLs. This document proposes a way to allow specifying multiple URLs
Copy link
Contributor

Choose a reason for hiding this comment

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

creating one CR for each repo?
is this a lot of work for this use case? or sharing the settings? across repos?

Copy link
Member Author

Choose a reason for hiding this comment

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

I listed this in the alternatives sections that we could potentially reuse the configmap features of that setting you implemented sometime ago and extend it, but that's an admin thing that doesn't allow much flexibility...

sharing setting is an interesting idea,having a configmap reference that would embed the value..

Copy link
Contributor

Choose a reason for hiding this comment

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

I think allowing multiple repo url could be a nice thing but if we allow regex then it might become quite complex as you have defined in scenarios
handling cases with multiple repos, what if a repo is edited and all

if the requirement for this design is to somehow share settings across repos then we could find someother way 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think there is flow in the scenarios i have listed, it's just a matter to have the webhook check if the new value (on update or creation) matches already and fail otherwise? is there more complexities to it do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah webhook can handle that.

wdym by multiples-url-to-the-same-repo?

if we go with regex https://github.com/foo/.*
how it could be same repo?
differen repos can also be in this case right 🤔

@chmouel chmouel changed the title Add design document Add design document for multiples-url-to-the-same-repo feature Apr 3, 2024
Start adding design document for the multiple-url-to-same-repo feature.

format of design may change in the future but this is good enough for a
start i believe.

Signed-off-by: Chmouel Boudjnah <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants