Skip to content

Conversation

@vladak
Copy link
Member

@vladak vladak commented Oct 20, 2020

workaround for #2528 - in order to prevent setuptools from reaching out to PyPi, the needed packages are installed first via pip. This is done so that it is possible to merge #3098.

Copy link
Contributor

@tulinkry tulinkry left a comment

Choose a reason for hiding this comment

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

The only thing I don't like that you use the requirements.txt file which is there kept for reference only and not really doing anything. Both of the requirements are now spread to two different places which is not ideal.

I think the goal would be to actually load the requirements in setup.py from the given files, so only one source of truth is available. I can try tonight if you're busy.

@vladak
Copy link
Member Author

vladak commented Oct 20, 2020

The only thing I don't like that you use the requirements.txt file which is there kept for reference only and not really doing anything. Both of the requirements are now spread to two different places which is not ideal.

Yeah, this is still confusing the heck out of me.

I think the goal would be to actually load the requirements in setup.py from the given files, so only one source of truth is available. I can try tonight if you're busy.

Ok, will wait for that.

@coveralls
Copy link

coveralls commented Oct 20, 2020

Pull Request Test Coverage Report for Build 5536

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 7 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.005%) to 75.693%

Files with Coverage Reduction New Missed Lines %
opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java 1 77.18%
opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/AnalyzerGuru.java 2 84.83%
opengrok-web/src/main/java/org/opengrok/web/api/v1/controller/FileController.java 4 53.49%
Totals Coverage Status
Change from base Build 5534: -0.005%
Covered Lines: 41399
Relevant Lines: 54693

💛 - Coveralls

@tulinkry
Copy link
Contributor

I've come up with f00a8ad - loading the requirements from file. But if you find a different way - possibly just using the requirements in setup.py, that would be much better. Perhaps tweaking the macosx env?

Because the goal of requirements.txt is to replicate the dev environment used for developing the package (pip freeze) while the requirements in setup.py are requirements to actually run the package (without specific version locks).

@vladak
Copy link
Member Author

vladak commented Oct 21, 2020

I had something like this in mind. As a workaround that would be acceptable I guess.

@vladak
Copy link
Member Author

vladak commented Oct 21, 2020

closing in favor of #3295

@vladak vladak closed this Oct 21, 2020
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.

3 participants