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

Add requirements-test.txt #5622

Merged
merged 1 commit into from
Oct 12, 2020
Merged

Add requirements-test.txt #5622

merged 1 commit into from
Oct 12, 2020

Conversation

drew2a
Copy link
Contributor

@drew2a drew2a commented Oct 8, 2020

The problem

We have two environments:

  • Pure Tribler
  • Test Tribler

Pure Tribler installs only necessary dependencies, they located in src/requirements.txt.

But there is no documentation and no requirements.txt for the Test Tribler.

Solution

I've added src/requirements-dev.txt and small instruction on how to run tests.

Maybe requirements-dev.txt is a bad name, and better to use requirements-test.txt.

What do you think?

@drew2a drew2a marked this pull request as ready for review October 8, 2020 17:52
src/requirements-dev.txt Outdated Show resolved Hide resolved
@kozlovsky
Copy link
Contributor

If these requirements are for tests specifically, I like the name requirements-test.txt better. But what about things like pre-commit, maybe it should also be specified? Then it probably should be requirements-dev.txt.

Maybe a good idea to specify all necessary tools for new developers to easily join the project.

@qstokkink
Copy link
Contributor

I'm impartial towards -test.txt or -dev.txt, both are used by other projects.

I would like to see the documentation in the docs/ folder though, fragmented documentation strewn about in README.md files in src subdirectories is very hard to maintain.

Nit: export PYTHONPATH=${PYTHONPATH}:echo {pyipv8,tribler-common,tribler-core,tribler-gui} | tr " " : does not work on Windows, we should make a Python script to launch the tests in an OS-independent manner.

@devos50
Copy link
Contributor

devos50 commented Oct 9, 2020

Note that I added run_tests.sh and run_tests.bat in the tribler/src directory recently.

src/README.md Outdated Show resolved Hide resolved
src/README.md Outdated Show resolved Hide resolved
src/requirements-dev.txt Outdated Show resolved Hide resolved
src/requirements-dev.txt Outdated Show resolved Hide resolved
src/requirements-dev.txt Outdated Show resolved Hide resolved
@drew2a drew2a changed the title Add requirements-dev.txt Add requirements-test.txt Oct 9, 2020
@drew2a
Copy link
Contributor Author

drew2a commented Oct 12, 2020

@kozlovsky I think it's a common courtesy to have such documentation in a project. I'm not sure we are the best persons for doing this, but we can try to write it.

@qstokkink good to know you are worried about documentation maintaining.

First of all: the most valuable part of this PR is requirements-test.txt, not documentation.

Next, I'm also worried about maintaining the documentation and I'd like to see the documentation changed every time the code changed. I understand that this is difficult. How to do this is less painful is to keep the documentation as close to the code as possible.

Note: current project documentation is quite outdated and fragmented.

xoriole
xoriole previously approved these changes Oct 12, 2020
kozlovsky
kozlovsky previously approved these changes Oct 12, 2020
@qstokkink
Copy link
Contributor

@qstokkink good to know you are worried about documentation maintaining.
[...]
Next, I'm also worried about maintaining the documentation and I'd like to see the documentation changed every time the code changed. I understand that this is difficult. How to do this is less painful is to keep the documentation as close to the code as possible.

Note: current project documentation is quite outdated and fragmented.

Historically, nobody gives a poop about the documentation. If you can somehow turn this around, I am all ears. Right now, I just perform a check-up manually every so often (apparently yearly): #2719, #3770, #3784, #4656.

@drew2a drew2a dismissed stale reviews from kozlovsky and xoriole via 9a86e62 October 12, 2020 09:23
qstokkink
qstokkink previously approved these changes Oct 12, 2020
@drew2a
Copy link
Contributor Author

drew2a commented Oct 12, 2020

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@drew2a drew2a merged commit a1e65dc into Tribler:devel Oct 12, 2020
@drew2a drew2a deleted the requirements-dev branch October 12, 2020 10:34
@qstokkink
Copy link
Contributor

@drew2a good point, we should probably mention the documentation here somewhere (at least): https://github.com/Tribler/tribler/blob/devel/doc/contributing.rst#pull-requests.

From your examples, this is the only one that I would actually expect developers to take the time to read: https://github.com/Privatix/dappctrl/blob/master/CONTRIBUTING.md#pull-request-process. It's categorized and concise. It would be cool if we could converge to something similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants