-
-
Notifications
You must be signed in to change notification settings - Fork 908
Make gleam.toml consistent #5300
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
Conversation
| let maybe_removed_item = toml["dependencies"] | ||
| .as_table_like_mut() | ||
| .and_then(|deps| deps.remove(package_to_remove)); | ||
|
|
||
| #[allow(clippy::indexing_slicing)] | ||
| let maybe_removed_dev_item = toml["dev-dependencies"] | ||
| .as_table_like_mut() | ||
| .and_then(|deps| deps.remove(package_to_remove)); | ||
|
|
||
| if maybe_removed_item.or(maybe_removed_dev_item).is_none() { | ||
| let remove = |toml: &mut toml_edit::DocumentMut, name| { | ||
| #[allow(clippy::indexing_slicing)] | ||
| toml[name] | ||
| .as_table_like_mut() | ||
| .and_then(|deps| deps.remove(package_to_remove)) | ||
| }; | ||
|
|
||
| // dev-dependencies is the old deprecated name for dev_dependencies | ||
| let removed = remove(&mut toml, "dependencies") | ||
| .or_else(|| remove(&mut toml, "dev_dependencies")) | ||
| .or_else(|| remove(&mut toml, "dev-dependencies")); | ||
|
|
||
| if removed.is_none() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this change the behaviour of this piece of code? Previously both the remove from dependencies and from dev_dependencies would run and then the check is performed. Does it make any difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does change in that way, but that config would have been invalid and rejected by the build tool
giacomocavalieri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Just one comment inline
These keys were inconsistent in that they used a different case convention to the other keys.
Assuming no bugs this is backwards compatible and will support the old config too.