⚠️ CONFLICT! Lineage pull request for: skeleton #44

cisagovbot · 2020-06-15T16:16:39Z

Lineage Pull Request: CONFLICT

Lineage has created this pull request to incorporate new changes found in an
upstream repository:

Upstream repository: https://github.com/cisagov/skeleton-generic.git
Remote branch: HEAD

Check the changes in this pull request to ensure they won't cause issues with
your project.

The lineage/skeleton branch has one or more unresolved merge conflicts
that you must resolve before merging this pull request!

How to resolve the conflicts

Take ownership of this pull request by removing any other assignees.

Clone the repository locally, and reapply the merge:

git clone git@github.com:cisagov/skeleton-python-library.git skeleton-python-library
cd skeleton-python-library
git remote add skeleton https://github.com/cisagov/skeleton-generic.git
git remote set-url --push skeleton no_push
git switch develop
git checkout -b lineage/skeleton --track origin/develop
git pull skeleton HEAD
git status

Review the changes displayed by the status command. Fix any conflicts and
possibly incorrect auto-merges.

After resolving each of the conflicts, add your changes to the branch,
commit, and push your changes:

git add requirements-test.txt requirements.txt 
git commit --message="Resolve Lineage conflicts."
git push --force --set-upstream origin lineage/skeleton

Wait for all the automated tests to pass.
Check the "Everything is cool" checkbox below:
- ✌️ The conflicts in this pull request have been resolved.
Mark this draft pull request "Ready for review".

Note: You are seeing this because one of this repository's maintainers has
configured Lineage to open pull requests.

For more information:

🛠 Lineage configurations for this project are stored in .github/lineage.yml

📚 Read more about Lineage

setuptools usually comes along with pip, but wheel does not. Using wheel where possible to build python extensions is more modern and more security conscious than using setup.py.

Add setuptools and wheel as pip dependencies

mcdonnnj

jsf9k

I have a question 🖐️

jsf9k · 2020-06-15T20:49:29Z

requirements-test.txt

@@ -1 +1,3 @@
+--requirement requirements.txt
 -e .[test]
+pre-commit


Should the pre-commit dependency be part of the test section in setup.py

pre-commit is in the test section, but I wasn't sure if we wanted to leave it in requirements-test.txt in case something changes with the configuration in setup.py. I'm happy to remove it from requirements-test.txt if we want to leave it to the setup.py configuration.

Let's remove it from requirements-test.txt so we have a single source of truth.

Please see 370f2a2 for that change.

jsf9k

See comments.

jsf9k · 2020-06-15T21:14:16Z

requirements.txt

@@ -1 +1,3 @@
 -e .
+setuptools


These requirements should be added to the main section of setup.py. If we have a setup.py file then the requirements*.txt files should just call that so we have a single source of truth.

I think wheel should remain in requirements.txt because it is an environment preference and not something required for the project to function. Per packaging documentation:

install_requires is a setuptools setup.py keyword that should be used to specify what a project minimally needs to run correctly.

I removed setuptools from requirements.txt in 370f2a2.

Can @felddy, @dav3r, and @hillaryj weigh in here? We should do the same thing everywhere, and @felddy and I added wheel to setup.py in cisagov/development-guide#21.

I have no preference here. @mcdonnnj makes a good point about wheel being a preference vs. being required, but I'm not sure it makes a big difference either way in this case. I'll go with whatever the group decides on this one.

@mcdonnnj makes a good point, but I also like having a single source of truth. I think mixing dependencies between requirements*.txt and setup.py files will eventually lead to confusion. I think simplicity trumps his argument in this case, but I will also concede to whatever the group decides.

I would like to point out that we already do this with requirements-dev.txt where ipython and semver are described as preferences for a development environment. They are not necessary for the functionality of the package, but are used for development.

OK, you have convinced me. Can you create a PR to do the same thing in cisagov/development-guide?

I approve this train of... thought. 🧠 🚂
ALL ABOARD THE BRAIN TRAIN!!! WOO WOO!

…ngle Source of Truth.

dav3r

☸️

jsf9k and others added 3 commits June 9, 2020 15:10

Add setuptools and wheel as pip dependencies

87ea530

setuptools usually comes along with pip, but wheel does not. Using wheel where possible to build python extensions is more modern and more security conscious than using setup.py.

Pull in requirements.txt from requirements-test.txt

7f8d430

Merge pull request #45 from cisagov/improvement/add-wheel

6668e3b

Add setuptools and wheel as pip dependencies

cisagovbot assigned dav3r, felddy, hillaryj, jsf9k and mcdonnnj Jun 15, 2020

mcdonnnj unassigned felddy, jsf9k, hillaryj and dav3r Jun 15, 2020

Resolve Lineage conflicts.

e211491

mcdonnnj force-pushed the lineage/skeleton branch from 6605b2c to e211491 Compare June 15, 2020 17:23

mcdonnnj requested review from jsf9k, dav3r, felddy and a team June 15, 2020 17:30

mcdonnnj marked this pull request as ready for review June 15, 2020 17:31

mcdonnnj requested review from hillaryj and mcdonnnj as code owners June 15, 2020 17:31

mcdonnnj approved these changes Jun 15, 2020

View reviewed changes

felddy approved these changes Jun 15, 2020

View reviewed changes

jsf9k requested changes Jun 15, 2020

View reviewed changes

Remove requirements that already exist in setup.py to align with a Si…

370f2a2

…ngle Source of Truth.

mcdonnnj requested a review from jsf9k June 15, 2020 21:36

felddy approved these changes Jun 16, 2020

View reviewed changes

jsf9k approved these changes Jun 16, 2020

View reviewed changes

dav3r approved these changes Jun 16, 2020

View reviewed changes

mcdonnnj merged commit 50b3841 into develop Jun 16, 2020

mcdonnnj deleted the lineage/skeleton branch June 16, 2020 15:22

⚠️ CONFLICT! Lineage pull request for: skeleton #44

⚠️ CONFLICT! Lineage pull request for: skeleton #44

Uh oh!

Conversation

cisagovbot commented Jun 15, 2020 • edited by felddy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Lineage Pull Request: CONFLICT

How to resolve the conflicts

Uh oh!

mcdonnnj left a comment

Choose a reason for hiding this comment

Uh oh!

jsf9k left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsf9k left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcdonnnj Jun 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dav3r left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cisagovbot commented Jun 15, 2020 •

edited by felddy

Loading

mcdonnnj Jun 15, 2020 •

edited

Loading