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

bin/dry-run.rb requires a development container to run #4215

Merged
merged 3 commits into from
Sep 10, 2021

Conversation

brrygrdn
Copy link
Contributor

@brrygrdn brrygrdn commented Sep 10, 2021

While it is possible to run bin/dry-run.rb on a native machine, it requires the user to correctly install native dependencies, such as the right versions of package managers and so on to be guaranteed to match a 'real' Dependabot run.

Since we use this tool as a first line debugging step, it often adds friction to the process of reproducing builds if a native environment is used.

In order to reduce context and make this process more confident, let's make it a requirement the script is ran using the developer shell so we default to reproducible outcomes.

@brrygrdn brrygrdn requested a review from a team as a code owner September 10, 2021 13:20
Dockerfile.development Outdated Show resolved Hide resolved
@brrygrdn brrygrdn force-pushed the brrygrdn/dry-run-requires-dev-shell branch from a6cb175 to 6e18a6f Compare September 10, 2021 13:24
bin/dry-run.rb Outdated
@@ -35,6 +35,15 @@

# rubocop:disable Style/GlobalVars

unless File.exist?("/etc/dependabot-core-dev")
Copy link
Member

Choose a reason for hiding this comment

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

WDYT about checking if the user is dependabot instead?

[dependabot-core-dev] ~/dependabot-core $ whoami
dependabot

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, I like that, it saves the hassle of having to rebuild images

Copy link
Member

@jurre jurre left a comment

Choose a reason for hiding this comment

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

Nice one! I made a suggestion that doesn't require touching a file, but this totally works also

@brrygrdn brrygrdn enabled auto-merge September 10, 2021 16:31
@brrygrdn brrygrdn merged commit 9311442 into main Sep 10, 2021
@brrygrdn brrygrdn deleted the brrygrdn/dry-run-requires-dev-shell branch September 10, 2021 16:59
@mctofu mctofu mentioned this pull request Sep 20, 2021
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.

2 participants