Skip to content

Add validation for crate/package name in new/init#1943

Merged
messense merged 5 commits intoPyO3:mainfrom
alonme:new-init-validate-name
Feb 27, 2024
Merged

Add validation for crate/package name in new/init#1943
messense merged 5 commits intoPyO3:mainfrom
alonme:new-init-validate-name

Conversation

@alonme
Copy link
Contributor

@alonme alonme commented Feb 17, 2024

partially solves #1398 (only cargo)

The current commit only adds a very basic validation as a POC before implementing the needed cargo / pypi checks
A few questions i have before finishing the implementation

  1. The cargo validation seems to be comprised of 2 main parts,
    a. Basic validation which is part of the crate cargo-util-schemas, and is not made public outside the crate

    b. Extra validations that are public from the cargo package

    I believe we should vendor these logics - one is private, and the other requires the cargo package which i believe will add many dependencies to the project

  2. The generate_project function is de-facto defining a default value for GenerateProjectOptions.name - would it be better to move this logic into GenerateProjectOptions?

@netlify
Copy link

netlify bot commented Feb 17, 2024

Deploy Preview for maturin-guide ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 9b21beb
🔍 Latest deploy log https://app.netlify.com/sites/maturin-guide/deploys/65d9fb0ee8676600086e03ee
😎 Deploy Preview https://deploy-preview-1943--maturin-guide.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@messense
Copy link
Member

  1. I believe we should vendor these logics - one is private, and the other requires the cargo package which i believe will add many dependencies to the project

Agreed.

2. The generate_project function is de-facto defining a default value for GenerateProjectOptions.name - would it be better to move this logic into GenerateProjectOptions?

Makes sense.

name,
);
}
if is_windows_reserved(name) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original code bailed if the platform is actually windows, I think this should be good enough for now

@alonme
Copy link
Contributor Author

alonme commented Feb 24, 2024

@messense I believe this is ready for review 😄

@messense messense self-requested a review February 27, 2024 06:23
@messense messense merged commit dd81231 into PyO3:main Feb 27, 2024
@alonme
Copy link
Contributor Author

alonme commented Feb 27, 2024

@messense Thanks!

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.

2 participants