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

Formatter doesn't seem to have appropriate exit codes #4102

Closed
1 task done
anicolao opened this issue Dec 24, 2022 · 4 comments
Closed
1 task done

Formatter doesn't seem to have appropriate exit codes #4102

anicolao opened this issue Dec 24, 2022 · 4 comments
Labels
invalid This doesn't seem right

Comments

@anicolao
Copy link

anicolao commented Dec 24, 2022

Environment information

CLI:
  Version:              11.0.0
  Color support:        true

Platform:
  CPU Architecture:     aarch64
  OS:                   macos

Environment:
  ROME_LOG_DIR:         unset
  NO_COLOR:             unset
  TERM:                 "xterm-256color"

Rome Configuration:
  Status:               loaded
  Formatter disabled:   false
  Linter disabled:      false

Workspace:
  Open Documents:       0

Discovering running Rome servers...

What happened?

  1. npx rome format source complains about some source files
  2. echo $? outputs 0

Expected result

I expect the exit code to indicate a problem so that scripting can react to an incorrectly formatted project.

Code of Conduct

  • I agree to follow Rome's Code of Conduct
@anicolao anicolao added the S-To triage Status: user report of a possible bug that needs to be triaged label Dec 24, 2022
@ematipico
Copy link
Contributor

That's intended. If you wish to take advantage of exit codes, rome ci is the command you want.

We might re evaluate exit codes in the future

@ematipico ematipico added invalid This doesn't seem right and removed S-To triage Status: user report of a possible bug that needs to be triaged labels Jan 6, 2023
@anicolao
Copy link
Author

anicolao commented Jan 6, 2023

Is there a design doc you can point me to that can help me understand why I would want to use a command line tool that is like no other I've used in 40 years?

@ematipico
Copy link
Contributor

Some issues you might want to follow up on:

We plan to change the behaviour of the command. The current behaviour is intended and not a bug.

@anicolao
Copy link
Author

anicolao commented Jan 6, 2023

Ah I am sorry I didn't see issue #3443 before, I should have found it before logging a duplicate. Very sloppy of me.

FWIW I think @JaySal's comment there is correct, that rome format should act as a pipeline command and print formatted output to stdout by default, and the verbose error message-style output belongs in some other command. But I don't care too much about this as rome format --write is more convenient than dealing with classic pipelines anyway, and not so far off the beaten path as to raise eyebrows.

As far as I'm concerned rome ci does too much and has no point in existing, when I could instead run rome lint followed by rome format or the other way around. As more tools get added to rome it is more and more likely that the users will want a subset of them, and not a single "does it all" ci command that then needs phases disabled with flags. Just my opinion.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants