Skip to content

Conversation

@guenp
Copy link

@guenp guenp commented Apr 3, 2021

@guenp guenp requested a review from anjbur April 3, 2021 02:39
Copy link
Member

@vxfield vxfield left a comment

Choose a reason for hiding this comment

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

Shouldn't we try to always use the latest version of these packages instead of pinning to a specific version?
This way if there are new security fixes we the packages we will automatically pick them. There is a risk of breaking changes, but then we can address them as they come, right?

@jenshnielsen
Copy link

@guenp and @vxfield its usually a bad idea to pin to an exact version (with ==) in setup.py/setup.cfg. It makes it much harder to deploy the package together with other packages having a requirement on the same package. I would suggest that you do pygments>=2.7.4

In QCoDeS we are using dependabot to keep the requirements that we perform most of the actual ci testing against (in requirements.txt) up to date and automatically open new pull requests when a new package is released. Meanwhile the requirements in setup.py/setup.cfg are normally the minimum supported versions of the required packages. In qcodes we are using requirements-builder to create an additional ci job that test against these. By being flexible on your enforced requirements you make it easier to deploy your package together with other packages.

@guenp
Copy link
Author

guenp commented Apr 12, 2021

Thanks @jenshnielsen, those are great ideas. I've created a new issue for setting up dependabot: #41. Jens' suggestion should fix the build error as well.

@vxfield
Copy link
Member

vxfield commented Apr 12, 2021

Now the build issue is unrelated to this PR but an issue with building the qsharp-runtime on Mac...

Copy link
Contributor

@anjbur anjbur left a comment

Choose a reason for hiding this comment

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

Confirmed that this resolves the Component Governance flags, looks good to me!

@guenp
Copy link
Author

guenp commented Apr 13, 2021

Restarted pipeline after microsoft/qsharp-runtime#623 was merged

@guenp guenp merged commit 4d19dec into main Apr 13, 2021
@vxfield vxfield deleted the guenp/address-CVE-alerts branch April 14, 2021 19:55
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.

5 participants