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

fix: 🪶 experiment normalised-upload-paths was left out of the list of available experiments #2076

Merged
merged 3 commits into from
May 10, 2023

Conversation

MatthewDolan
Copy link
Contributor

We are seeing this warning in our Buildkite jobs:

WARN   Unknown experiment enabled: "normalised-upload-paths"

It looks like the experiment is present and does do something.

I would hypothesize that this is just an accidental oversight to leave this out of the list.

Copy link
Contributor

@moskyb moskyb left a comment

Choose a reason for hiding this comment

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

ah, i can't believe i missed this! thanks so much @MatthewDolan, nice catch

@@ -1308,7 +1308,7 @@ func (b *Bootstrap) defaultCheckoutPhase(ctx context.Context) error {
var mirrorDir string

// If we can, get a mirror of the git repository to use for reference later
if experiments.IsEnabled(`git-mirrors`) && b.Config.GitMirrorsPath != "" && b.Config.Repository != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

aha, i was foolish in searching only for "git-mirrors". nice catch!

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, bad habit of mine is using `` for const-ish strings.

@moskyb
Copy link
Contributor

moskyb commented May 10, 2023

I would hypothesize that this is just an accidental oversight to leave this out of the list.

you would hypothesize correctly 😅

@moskyb moskyb enabled auto-merge May 10, 2023 03:36
auto-merge was automatically disabled May 10, 2023 03:56

Head branch was pushed to by a user without write access

@MatthewDolan
Copy link
Contributor Author

Sorry. I fixed on last typo I noticed in the variable name (I had used the American spelling) so it disabled auto-merge. Thanks for the review!

@moskyb
Copy link
Contributor

moskyb commented May 10, 2023

ah thank you!

@moskyb moskyb enabled auto-merge May 10, 2023 22:24
@moskyb moskyb merged commit f93e61c into buildkite:main May 10, 2023
@lox
Copy link
Contributor

lox commented May 10, 2023

Any chance of getting a release of this out ASAP @moskyb?

@MatthewDolan MatthewDolan deleted the dolan-2023-05-09-fix-warning branch May 11, 2023 04:06
@moskyb
Copy link
Contributor

moskyb commented May 17, 2023

gidday @lox, we're gonna cut a release once #2040 is done, probably within the next week or so

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