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

removed tests/scripts folder with its contents #7680

Merged
merged 6 commits into from
Feb 2, 2020
Merged

Conversation

Bhavam
Copy link
Contributor

@Bhavam Bhavam commented Feb 2, 2020

Fixes #7659
Hi @pradyunsg are the changes as per requirement and if you can give any help on if I need to change anything for the failing tests.

@pradyunsg
Copy link
Member

Thanks @Bhavam! This looks good to me.

Your PR seems to be missing a news file, as documented here. It's basically a file in the news/ folder with the appropriate name and a one line description.

Could you please a news entry associated with this PR?

@pradyunsg
Copy link
Member

pradyunsg commented Feb 2, 2020

@Bhavam you'd want to run tox -e lint locally. There are code formatting tools that may run as a part of that (you'd want to commit those changes) and code quality checkers that would fail if the code does not meet those checks.

Looks like the only change being made as part of tox -e lint is adding a newline at the end of the file. You can probably just run tox -e lint and commit those changes to make things work. :)

@chrahunt
Copy link
Member

chrahunt commented Feb 2, 2020

Can we also remove the scripts/ bullet point under test/ from the developer docs?

Copy link
Member

@chrahunt chrahunt left a comment

Choose a reason for hiding this comment

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

One more comment otherwise this looks good to me.

news/7680.removal Outdated Show resolved Hide resolved
@chrahunt chrahunt merged commit 7534dcc into pypa:master Feb 2, 2020
@chrahunt
Copy link
Member

chrahunt commented Feb 2, 2020

Thank you for your help @Bhavam

@xavfernandez
Copy link
Member

Shouldn't the news entry have been a trivial one ?

@chrahunt
Copy link
Member

chrahunt commented Feb 3, 2020

It could go either way. I figure if someone was actually using this script, now they can just grep for it in the repo and see what happened vs git log -G.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Mar 10, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor and remove tests/scripts/test_all_pip.py
4 participants