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

www: Make Builder related URLs take buildername #7549

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tdesveaux
Copy link
Contributor

@tdesveaux tdesveaux commented May 1, 2024

This allow to share user-friendly URLs between users.

Note that using buildername in APIs can be problematic, see #7552.

Contributor Checklist:

  • I have updated the unit tests
  • I have created a file in the newsfragments directory (and read the README.txt in that directory)
  • [N/A] I have updated the appropriate documentation

@tdesveaux tdesveaux marked this pull request as draft May 2, 2024 06:26
@tdesveaux tdesveaux force-pushed the react-base/buildername-url-router branch from f844a12 to 6f6c12f Compare May 2, 2024 06:52
@tdesveaux tdesveaux marked this pull request as ready for review May 2, 2024 06:55
@tdesveaux tdesveaux force-pushed the react-base/buildername-url-router branch from 6f6c12f to f1fef41 Compare May 5, 2024 06:30
@p12tic
Copy link
Member

p12tic commented May 6, 2024

The problem with builder names in URLs is that builder names can change and then all URLs would break. I will think about how this could be solved. The PR itself looks good and would improve user experience.

@tdesveaux
Copy link
Contributor Author

The problem with builder names in URLs is that builder names can change and then all URLs would break. I will think about how this could be solved. The PR itself looks good and would improve user experience.

I agree that it can be an issue (in addition to #7552).
I looked into this as it was a feedback from users on things they missed since upgrading from 0.8.

But, maybe I missed something, but builderid seem to not be stable either in case of name change right?
I tried with the default sample from a create-master, ran a build then renamed runtests to run-tests and the BuilderId changed.
So wouldn't buildernames be as faulty as builderids in urls?

@p12tic
Copy link
Member

p12tic commented May 6, 2024

So wouldn't buildernames be as faulty as builderids in urls?

You're completely right. Buildername has 1:1 relationship with builderid. Which means that there are no blockers for this PR.

@p12tic p12tic force-pushed the react-base/buildername-url-router branch from f1fef41 to 670dc58 Compare May 6, 2024 18:23
@tdesveaux
Copy link
Contributor Author

Still think merging this should be on hold until #7552 is addressed as this would add onto the inconsistency of which builder can be accessed by name.

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.

None yet

2 participants