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

Update README.md #7

Closed
wants to merge 1 commit into from
Closed

Update README.md #7

wants to merge 1 commit into from

Conversation

volkancakil
Copy link

No description provided.

@dannymcgee
Copy link
Contributor

@volkancakil Thanks for the PR!

Kebab case is totally acceptable for package names (there's a whole bikeshed about it here if you're interested). My thinking was that snake case is pretty unusual for Nx project names, and since Rust doesn't have an (official) opinion, it would be better to stick with the conventions of the rest of the workspace where possible.

That said, I'm not married to it, so if that argument didn't sway you, feel free to update the other kebab-case references in the readme and I'd be happy to accept this. Also, if you wouldn't mind adding an e2e test case for a snake-case project name I would appreciate it — I kind of doubt Nx would break over it, but stranger things have happened. 🙂

@volkancakil
Copy link
Author

@dannymcgee Hello
I made this PR because generator adds snake_case project names to the workspace and that means nx would not find corresponding project and fail.
As a solution we might edit projects field on workspace.json manually or use snake_case paramaters with nx commands (cargo new package-name changes - with _ by default too)

Have a nice day.

@volkancakil
Copy link
Author

volkancakil commented Jan 28, 2022

this nrwl util function at line 52 returning constantName as a snake_case string and we are using that variable as a project name at line 65 , i guess this is the problem.

@dannymcgee
Copy link
Contributor

@volkancakil

Hello I made this PR because generator adds snake_case project names to the workspace and that means nx would not find corresponding project and fail. As a solution we might edit projects field on workspace.json manually or use snake_case paramaters with nx commands (cargo new package-name changes - with _ by default too)

Damn, good catch — I guess I forgot how I actually implemented that. I'll take a closer look at this first thing in the morning, thanks again!

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