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

Delete leftover build script #7611

Merged
merged 2 commits into from
Jul 25, 2023
Merged

Delete leftover build script #7611

merged 2 commits into from
Jul 25, 2023

Conversation

jeffwidman
Copy link
Member

This script is a leftover from when we had a monolithic image for all ecosystems.

Now that we use a specific docker image just for python, that dockerfile uses the parameterized version of this script, called build_for_version.

You can see more historical context in this commit: 3e99935

I grep'd around and couldn't find any remaining usages of the build script, so let's nuke it.

Copy link
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

If you change a dependency and then manually run the build script, will you want all native helpers reinstalled for tests to pass? I guess that's why the previous build script installed for all pythons?

@jeffwidman
Copy link
Member Author

I guess that's why the previous build script installed for all pythons?

I don't think so... this commit explains that the build script was left-in for the monolithic image... probably it was not removed when the multi-stage build was added in order to reduce risk during initial rollout, and only later once everything was proven stable was it removed but removing this script was overlooked.

If you change a dependency and then manually run the build script, will you want all native helpers reinstalled for tests to pass?

Technically that's true and this would be a convenience, but TBH it's a pain to have to keep the list of python versions updated here... Historically the frequency that we need to re-run the build script for testing seems like it's smaller than the frequency of bumping python releases.

It also feels dangerous to have a build script that someone might use during dev flow that's different than the build-for-helpers script that is actually used in prod.

This script is a leftover from when we had a monolithic image for all
ecosystems.

Now that we use a specific docker image just for python, that dockerfile
uses the parameterized version of this script, called
`build_for_version`.

You can see more historical context in this commit: 3e99935

I grep'd around and couldn't find any remaining usages of the `build`
script, so let's nuke it.
All our other helper build scripts are simply called `build`. So now
that this is the only python build script, we no longer need to
disambiguate it from the other build script... so let's also rename it
to just `build` for consistency with the other ecosystems.

Split out as a separate commit from the prior one in order to make the
`git` trail easier to follow.
@jeffwidman jeffwidman force-pushed the remove-leftover-build-script branch from 1665b0d to 0c252d0 Compare July 25, 2023 17:26
@jeffwidman jeffwidman merged commit e4afdd4 into main Jul 25, 2023
91 checks passed
@jeffwidman jeffwidman deleted the remove-leftover-build-script branch July 25, 2023 17:49
brettfo pushed a commit to brettfo/dependabot-core that referenced this pull request Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants