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

Escape label names with URL characters #1297

Merged
merged 3 commits into from
May 24, 2022

Conversation

Fryguy
Copy link
Contributor

@Fryguy Fryguy commented Oct 28, 2020

Fixes #1296

@tarebyte This is WIP because I am having trouble recording the VCR cassettes, and I'm not sure what I'm doing wrong. I could probably use some pointers there.

@Fryguy

This comment has been minimized.

@Fryguy Fryguy force-pushed the fix_labels_with_url_chars branch from 89a7f12 to 91e0d4d Compare October 28, 2020 17:51
@Fryguy

This comment has been minimized.

@Fryguy Fryguy force-pushed the fix_labels_with_url_chars branch from 91e0d4d to fcd8564 Compare October 28, 2020 20:09
@Fryguy Fryguy changed the title [WIP] Escape label names with URL characters Escape label names with URL characters Oct 28, 2020
@Fryguy
Copy link
Contributor Author

Fryguy commented Oct 28, 2020

@tarebyte This is ready to go!

@Fryguy
Copy link
Contributor Author

Fryguy commented Jan 5, 2021

@tarebyte Is this good to merge?

@Fryguy
Copy link
Contributor Author

Fryguy commented Mar 5, 2021

@tarebyte Are you good with this change?

@Fryguy
Copy link
Contributor Author

Fryguy commented Sep 17, 2021

@tarebyte @hmharvey Bump?

@Fryguy Fryguy force-pushed the fix_labels_with_url_chars branch from fcd8564 to 1775e35 Compare November 22, 2021 21:10
@Fryguy
Copy link
Contributor Author

Fryguy commented Nov 22, 2021

Rebased against 4-stable to verify on the new 3.0 test suite/

@tarebyte Is there any else you need me to do here to get this in? Should it go against a different branch or multiple branches?

Copy link
Contributor

@nickfloyd nickfloyd left a comment

Choose a reason for hiding this comment

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

Hey, @Fryguy thanks so much for this change ❤️ and for using erb which more closely aligns with the encoding spec!

@nickfloyd nickfloyd merged commit 519c913 into octokit:4-stable May 24, 2022
@Fryguy Fryguy deleted the fix_labels_with_url_chars branch May 24, 2022 17:04
@nickfloyd nickfloyd added the bug label May 24, 2022
Fryguy added a commit to Fryguy/manageiq-release that referenced this pull request May 26, 2022
@nickfloyd nickfloyd added Type: Bug Something isn't working as documented and removed bug labels Oct 28, 2022
Fryguy added a commit to ManageIQ/multi_repo that referenced this pull request Mar 24, 2023
Fryguy added a commit to ManageIQ/multi_repo that referenced this pull request Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If a label contains a ?, update_label silently fails
2 participants