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

rome ci does not care about rome.json #3167

Closed
1 task done
anonrig opened this issue Sep 6, 2022 · 3 comments · Fixed by #3171
Closed
1 task done

rome ci does not care about rome.json #3167

anonrig opened this issue Sep 6, 2022 · 3 comments · Fixed by #3171
Assignees
Labels
A-Core Area: core S-Bug: confirmed Status: report has been confirmed as a valid bug

Comments

@anonrig
Copy link
Contributor

anonrig commented Sep 6, 2022

Environment information

node 18.8.0
macos latest version
rome 0.9.0 - installed using pnpm add -D rome@next

What happened?

npm run format:ci which is rome ci . does not care about rome.json configuration.

there isn't any clear way of running formatter on CI and throwing errors if it fails.

{
  "formatter": {
    "indentStyle": "space",
    "indentSize": 2
  },
  "linter": {
    "enabled": false
  }
}

Screen Shot 2022-09-06 at 12 29 02 PM

Expected result

It should care linter enabled false configuration and not check for linting.

Code of Conduct

  • I agree to follow Rome's Code of Conduct
@anonrig anonrig added the S-To triage Status: user report of a possible bug that needs to be triaged label Sep 6, 2022
@MichaReiser MichaReiser added S-Bug: confirmed Status: report has been confirmed as a valid bug S-To triage Status: user report of a possible bug that needs to be triaged and removed S-To triage Status: user report of a possible bug that needs to be triaged labels Sep 6, 2022
@MichaReiser
Copy link
Contributor

It seems that the ci command doesn't respect the configuration all together as it also doesn't take formatting options into account. @ematipico @leops would you be able to have a look and I think we should consider doing a follow up release with the fix included

@MichaReiser MichaReiser added A-Core Area: core and removed S-To triage Status: user report of a possible bug that needs to be triaged labels Sep 6, 2022
@anonrig
Copy link
Contributor Author

anonrig commented Sep 6, 2022

You can reproduce the same behaviour with this pull request: anonrig/fast-querystring#11

@ematipico ematipico self-assigned this Sep 7, 2022
@ematipico ematipico moved this to In Progress in Rome 2022 Sep 7, 2022
@leops
Copy link
Contributor

leops commented Sep 7, 2022

I've noted two slightly unrelated bugs here:

  • From what I can see the configuration file is correctly being loaded (changing the indent size / style in the rome.json works as expected for instance), but the ci command is hard-coded to call into both the formatter and the linter and doesn't respect the enabled setting for either tool. This should be fixed at two points: the Workspace should return an error the the linting or formatting methods are called but the corresponding tool is disabled, and the CLI should not attempt to call into these methods in the first place if the supports_feature method has returned false / Some(_)
  • The CLI flags like --indent-style are being ignored, specifically they get read into a WorkspaceSettings struct but this information never gets used or applied anywhere

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Core Area: core S-Bug: confirmed Status: report has been confirmed as a valid bug
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants