Skip to content
This repository has been archived by the owner on Aug 4, 2022. It is now read-only.

Add pip install commands to tox environments. #97

Merged
merged 6 commits into from
Nov 2, 2020
Merged

Conversation

b4handjr
Copy link
Collaborator

@b4handjr b4handjr commented Oct 23, 2020

Step one of #93

@hackebrot
Copy link
Collaborator

Thanks for working on this, @jrbenny35! I'll review your changes on Monday. 📝

@hackebrot hackebrot added the code quality Tasks related to linting, coding style, type checks label Oct 23, 2020
@hackebrot
Copy link
Collaborator

I'm wondering why GitHub doesn't show checks from CircleCI. 🤔

@jklukas
Copy link
Contributor

jklukas commented Oct 26, 2020

I'm wondering why GitHub doesn't show checks from CircleCI.

This isn't obvious to me. I checked configuration compared to bigquery-etl, and I don't see any significant difference that would explain this. I don't remember having to follow an special steps on other repositories once CircleCI was successfully running the checks.

@willkg
Copy link
Member

willkg commented Oct 26, 2020

I think you set it in the GitHub settings for the project. In Settings, click on the Branches tab, then select the branch (main or master) in the protected branches list, then select which status checks have to pass.

@hackebrot
Copy link
Collaborator

I just updated the settings for the main branch to require status checks. The UI shows this warning:

No status checks found
Sorry, we couldn’t find any status checks in the last week for this repository.

I'm pretty sure we had status checks from CircleCI before and the CircleCI dashboard shows that it's still running the checks.

@willkg
Copy link
Member

willkg commented Oct 26, 2020

In GitHub Settings, under Webhooks, you can verify that CircleCI is in the list and if it is, you can send some payloads and I think that'll trigger checks to be recognized.

@hackebrot
Copy link
Collaborator

Hey folks! 👋

Ben and I discovered this week that adding a pip install to the commands does not ensure we get compatible packages, because tox first installs the packages in deps and then installs the burnham sdist before executing the commands section.

I had another look at this today and may have found a way to make sure we test the same packages in tox as we install in Dockerfile while ensuring compatibility between burnham requirements and development requirements by using pip-tools' layered requirements feature. Note that the commands section will only be executed if commands_pre is successful.

Do we need to update our Dependabot configuration to receive updates for all dependencies? 📦

Please let me know your thoughts!

@b4handjr
Copy link
Collaborator Author

b4handjr commented Nov 2, 2020

r+, I think dependabot could update all of them but we may need to put them in a requirements dir. But looking through the dependabot code, it should find anything with a .txt and try to read it.

@hackebrot
Copy link
Collaborator

That sounds good! Thank you for the review @jrbenny35. Much appreciated! 🙇

@hackebrot hackebrot merged commit 66a9526 into main Nov 2, 2020
@hackebrot hackebrot deleted the code_checks_step_1 branch November 2, 2020 17:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
code quality Tasks related to linting, coding style, type checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants