Skip to content

Conversation

@blfpd
Copy link

@blfpd blfpd commented Feb 9, 2022

This fix is for projects that uses a requirements folder with files named base.txt, production.txt and such.

Fixes #18498.

@blfpd blfpd force-pushed the requirements-filename-patterns branch from ab4238a to 2719efe Compare February 10, 2022 11:11
@karthiknadig karthiknadig added the skip package*.json package.json and package-lock.json don't both need updating label Feb 10, 2022
package.json Outdated
Comment on lines 1577 to 1578
"**/*requirements{/**,*}.{txt,in}",
"**/*constraints{/**,*}.txt"
Copy link
Member

Choose a reason for hiding this comment

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

We have to be careful to not over-fit on file patterns as we don't want to try and highlight files that are not requirements files.

Suggested change
"**/*requirements{/**,*}.{txt,in}",
"**/*constraints{/**,*}.txt"
"*-requirements.{txt, in}",
"*-contstraints.txt",
"requirements-*.{txt, in}",
"constraints-*.txt",
"requirements/*.{txt,in}",
"constraints/*.txt"

Let us know if this still highlights files as expected for you.

Copy link
Author

Choose a reason for hiding this comment

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

I made a counter suggestion.

@brettcannon
Copy link
Member

@batisteo any chance you can look at my proposed changes this week? We are feature freezing at the end of the week.

@blfpd blfpd force-pushed the requirements-filename-patterns branch from 2719efe to 47eebb6 Compare February 16, 2022 07:44
@blfpd
Copy link
Author

blfpd commented Feb 16, 2022

Thanks @brettcannon for the heads up!
The changes didn’t work for me, it seems **/ is needed at the beginning of the pattern.

What do you think?

@brettcannon brettcannon reopened this Feb 16, 2022
@brettcannon brettcannon merged commit 24297ad into microsoft:main Feb 16, 2022
@brettcannon
Copy link
Member

@batisteo thanks!

"requirements-*.txt"
],
"**/*-requirements.{txt, in}",
"**/*-contstraints.txt",
Copy link

Choose a reason for hiding this comment

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

Typo here:

contstraints -> constraints

Copy link

Choose a reason for hiding this comment

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

Fixed in #18646, thanks for bringing to our attention.

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

Labels

skip package*.json package.json and package-lock.json don't both need updating

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhancement: Better detection of pip-requirement files

5 participants