-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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(config): user-agent properly shows ci #3754
Conversation
@wraithgar can we also write an E2E/smoke test that ensures this is sent to the registry properly when running a command w/ |
54087b0
to
0de45b8
Compare
As long as we still send a valid |
Good point, the env one should ... probably be flattened? |
i verified with |
added a smoke test, it's a bit hacky but it gets the job done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks very much correct. smoke test was added too so 👍
The way we were flattening user-agent back into itself meant that any values that were dependent on other config items would never be seen, since we have to re-flatten the item for each one it depends on. We also weren't re-flattening the user-agent when setting workspaces or workspace, which were things that affected the final result. This does change the main config value of `user-agent` but not the flattened one. We are not using the main config value anywhere (which is correct). PR-URL: #3754 Credit: @wraithgar Close: #3754 Reviewed-by: @nlf
39b3ba2
to
b4aac34
Compare
The way we were flattening user-agent back into itself meant that any
values that were dependent on other config items would never be seen,
since we have to re-flatten the item for each one it depends on.
We also weren't re-flattening the user-agent when setting workspaces or
workspace, which were things that affected the final result.
This does change the main config value of
user-agent
but not theflattened one. We are not using the main config value anywhere (which
is correct).