Project

Profile

Help

Issue #5074

closed

bootstrap.sh --travis depends on black

Added by mdellweg over 4 years ago. Updated over 4 years ago.

Status:
CLOSED - WORKSFORME
Priority:
Normal
Assignee:
-
Category:
-
Sprint/Milestone:
-
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version:
Platform Release:
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Plugin Template
Sprint:
Sprint 56
Quarter:

Description

When updating the travis infrastructure via bootstrap.sh, it should verify (or ensure) that black is listed in test-requirements.txt

Actions #1

Updated by daviddavis over 4 years ago

Is this strictly limited to black or do we also have this problem for other dependencies like coverage and flake8?

Actions #2

Updated by mdellweg over 4 years ago

daviddavis wrote:

Is this strictly limited to black or do we also have this problem for other dependencies like coverage and flake8?

I have not checked, but looking at the mechanism, i sure think so.

Actions #3

Updated by amacdona@redhat.com over 4 years ago

  • Triaged changed from No to Yes
  • Sprint set to Sprint 55
  • Tags Plugin Template added
Actions #4

Updated by daviddavis over 4 years ago

I think this task is problematic. Take these scenarios for example:

- A plugin writer doesn't store their deps in test_requirements.txt but maybe uses a different file name
- A plugin writer is requiring a package like black indirectly. For example, they might be using flake8-black which requires black.

We provide a base set of packages when bootstrapping a plugin and I think that should suffice. It's pretty obvious when you're missing a dependency and it's easy to fix. I don't think we should check that the package is being required somehow when updating a plugin's files. It's not worth the effort.

Actions #5

Updated by bmbouter over 4 years ago

@daviddavis, I agree w/ your reasoning here. Does the test-requirements.txt in the plugin_template have black in it, itself?

Actions #6

Updated by bmbouter over 4 years ago

I see it in here so I think we're ok without the additional check.

Actions #7

Updated by daviddavis over 4 years ago

Yes, and we have it for plugins too. It gets included by flake8-black which depends on black.

https://github.com/pulp/plugin_template/blob/master/templates/test/test_requirements.txt.j2#L4

Actions #8

Updated by dkliban@redhat.com over 4 years ago

  • Sprint changed from Sprint 55 to Sprint 56
Actions #9

Updated by daviddavis over 4 years ago

  • Status changed from NEW to CLOSED - WORKSFORME

I'm setting this to WONTFIX but I am happy to discuss further if others disagree.

Also available in: Atom PDF