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 the option to change bonfire reservation duration #29

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

jrusz
Copy link
Contributor

@jrusz jrusz commented Oct 12, 2023

Sometimes our pr_check can take over an hour so we need the option to reserve the namespace for longer.

@jrusz
Copy link
Contributor Author

jrusz commented Oct 12, 2023

Not sure who to ask for review, @Victoremepunto could you maybe review?

@@ -16,7 +16,7 @@ source ${CICD_ROOT}/_common_deploy_logic.sh
# -> use this PR as the template ref when downloading configurations for this component
# -> use this PR's newly built image in the deployed configurations
set -x
export NAMESPACE=$(bonfire namespace reserve --pool ${NAMESPACE_POOL})
Copy link
Contributor

Choose a reason for hiding this comment

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

if we want to add this feature, let's try follow the same approach (and syntax) for other global vars , see for example, line 13:

# Whether or not to deploy frontends (default: false)
: ${DEPLOY_FRONTENDS:="false"}

so something like a comment, a default value, then just use it on line 19

Also would tag @bsquizz here for review too ^

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, good suggestion @Victoremepunto -- having all the "options" defined up top is preferred for readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah sorry I just basically copied the same change I made in Gitlab includes. Now it looks more like the rest.

@jrusz jrusz force-pushed the bonfire-reservation branch from 7ccc703 to ffd396a Compare October 17, 2023 07:06
@Victoremepunto Victoremepunto merged commit 9d7af6e into RedHatInsights:main Oct 18, 2023
3 checks passed
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.

3 participants