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.

Sign up for GitHub

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

Jump to bottom

fix allowed change ref types environment read #1253

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

fix allowed change ref types environment read #1253

wants to merge 6 commits into from

Conversation

aoteropas
Copy link

@aoteropas aoteropas commented Dec 4, 2023 •
edited by BraisVQ
Loading

change the name of the variable in the if clause for the one that contains the env values. Follow the same approach that other Env Variables

Closes #1252

@aoteropas
fix allowed change ref types environment read
28f3094 
change the name of the variable in the if clause for the one that contains the env  values.
Follow the same approach that with other Env Variables
michaelsauter
michaelsauter previously approved these changes Dec 4, 2023
View reviewed changes
@BraisVQ BraisVQ self-requested a review December 4, 2023 10:18
@BraisVQ
Copy link
Contributor

BraisVQ commented Dec 4, 2023

This change now make Webhook Proxy load the modified parameters but something is not working properly just yet

2023/12/04 10:03:07 zCNVmE Skipping change ref type BRANCH as ALLOWED_CHANGE_REF_TYPES does not include it
2023/12/04 10:03:59 PFYwTB -----
2023/12/04 10:03:59 PFYwTB Skipping change ref type TAG as ALLOWED_CHANGE_REF_TYPES does not include it

@aoteropas
set to lower the request chage type to match the one stored
e66e2ec 
when reading the env variable it is set to lower to unify the values. 
setting to lower the request information to ensure that comparison works
jafarre-bi
jafarre-bi requested changes Dec 5, 2023
View reviewed changes
Copy link
Contributor

@jafarre-bi jafarre-bi left a comment

Choose a reason for hiding this comment

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

If these values are turned lowercase, allowedChangeRefTypesDefault should also have a lowercase value. Otherwise, the fix looks fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jafarre-bi jafarre-bi jafarre-bi requested changes

@michaelsauter michaelsauter michaelsauter left review comments

@oalyman oalyman Awaiting requested review from oalyman oalyman is a code owner

@BraisVQ BraisVQ Awaiting requested review from BraisVQ

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error adding ALLOWED_CHANGE_REF_TYPES as env variable
4 participants
@aoteropas @BraisVQ @michaelsauter @jafarre-bi