Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_cli): Add a new option --config-path to set the path to rome.json #4158

Merged
merged 10 commits into from
Feb 13, 2023

Conversation

realtimetodie
Copy link
Contributor

@realtimetodie realtimetodie commented Jan 16, 2023

Adds a new option --config-path to set the filesystem path to the directory of the rome.json configuration file.

Example

$ rome check --config-path=../../../example

The --config option appends a base path to the rome.json configuration file path

../../../example/rome.json

Error handling

If the user explicitly sets a path to the rome.json configuration file using the --config-path option and the file is not found, an error will be thrown.

Example

$ rome check --config-path=/test.json

Error message

...
× Rome couldn't read the following file, maybe for permissions reasons or it doesn't exists: /test.json/rome.json
  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Jan 16, 2023

Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 970ab42
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/63ea7ad013208a0008d6b9c0

@realtimetodie realtimetodie force-pushed the feat-config-path branch 2 times, most recently from b4b4ef2 to 2eda94a Compare January 16, 2023 14:36
Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Thank you for this PR! I think we should rename the option, for example --config-path. This would be in line with --file-path

@realtimetodie realtimetodie changed the title feat(rome_cli): Add a new option --config to set the path to rome.json feat(rome_cli): Add a new option --config-path to set the path to rome.json Feb 12, 2023
@realtimetodie
Copy link
Contributor Author

@ematipico Yes, I like this idea. I renamed the option to --config-path and updated the documentation.

Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Have you tried running the CLI using windows like style paths? E.g. --config-path=..\foo\rome.json

Does it work?

crates/rome_service/src/configuration/mod.rs Outdated Show resolved Hide resolved
website/src/pages/cli.mdx Outdated Show resolved Hide resolved
crates/rome_cli/src/configuration.rs Outdated Show resolved Hide resolved
@realtimetodie
Copy link
Contributor Author

Have you tried running the CLI using windows like style paths? E.g. --config-path=..\foo\rome.json

The dot notation is covered by the DOS specification

assert_eq!(Path::new("..\\..").join("rome.json"), PathBuf::from("..\\..\\rome.json"));

@ematipico
Copy link
Contributor

@realtimetodie could you please tick the checkbox of The PR requires documentation? We would need to update the website with this new argument

@realtimetodie
Copy link
Contributor Author

realtimetodie commented Feb 13, 2023

@ematipico Yes, I will take care of the documentation.

@realtimetodie
Copy link
Contributor Author

@ematipico #4158 (comment)

@ematipico ematipico merged commit bb3cbc1 into rome:main Feb 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants