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.

Sign up for GitHub

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

Jump to bottom

fix: make the messages more meaningful #25

Merged
merged 5 commits into from
Mar 7, 2019
Merged

fix: make the messages more meaningful #25

merged 5 commits into from
Mar 7, 2019

Conversation

varl
Copy link
Contributor

@varl varl commented Mar 7, 2019

Fixes #24

@varl varl requested a review from ismay March 7, 2019 11:10
@varl
Copy link
Contributor Author

varl commented Mar 7, 2019 •
edited
Loading 

.../dhis2/libs/cli-style > ./bin/d2-style js check
No staged files found. 
No files to check. 

.../dhis2/libs/cli-style > ./bin/d2-style js check --all
Files to style: 
src/cmds/js_cmds/check.js 
 
[ERROR] 1 file(s) failed the style check 

.../dhis2/libs/cli-style ! > ./bin/d2-style js apply
No staged files found. 
No files to format. 
Stage 0 file(s). 

.../dhis2/libs/cli-style > ./bin/d2-style js apply --all
Stage 1 file(s). 
Staging /home/varl/dev/dhis2/libs/cli-style/src/cmds/js_cmds/check.js...

@ismay
Copy link
Contributor

ismay commented Mar 7, 2019 •
edited
Loading

Ah nice. So what would I get now with .../dhis2/libs/cli-style > ./bin/d2-style js check --all when it's checked all files and they're all fine?

I like the way lint-staged does it for example:

screenshot 2019-03-07 at 13 41 08

If everything's fine it clearly shows you which checks it ran, and that everything succeeded. That helps me as a user know that it checked everything and it was all fine. That's what confused me a bit about cli-style's original message

@varl
Copy link
Contributor Author

varl commented Mar 7, 2019

I missed that case:

.../dhis2/libs/cli-style ! > ./bin/d2-style js check --all
15 file(s) passed the style check.

@varl
Copy link
Contributor Author

varl commented Mar 7, 2019 •
edited
Loading

We are going to take a look at general guidelines for how we should present output from d2 command modules so the scope for this is just to get the messages good enough.

Eventually we want to make the output consistent and nice between all d2 commands and modules.

@ismay
Copy link
Contributor

ismay commented Mar 7, 2019

.../dhis2/libs/cli-style ! > ./bin/d2-style js check --all
15 file(s) passed the style check.

Nice, yeah that's what I expected 👍

@varl varl merged commit d2ae8af into master Mar 7, 2019
@varl varl deleted the no-files-to-style branch March 7, 2019 12:50
dhis2-bot pushed a commit that referenced this pull request Mar 25, 2019
chore(release): cut 2.0.0 [skip ci]
154e170 
# [2.0.0](v1.7.0...v2.0.0) (2019-03-25)

### Bug Fixes

* add 'target' to blacklisted folders ([#9](#9)) ([d6170b9](d6170b9))
* add apply/check command to js ([3da7d5c](3da7d5c))
* change to debug msg ([5be1ad2](5be1ad2))
* clean up js filter mechanism ([#21](#21)) ([ece8c53](ece8c53))
* do not put a scary error message when checking files ([#27](#27)) ([4cecb27](4cecb27))
* make the messages more meaningful ([#25](#25)) ([d2ae8af](d2ae8af))
* only the apply command was properly whitelisted ([#19](#19)) ([8a7556a](8a7556a))
* only whitelist js under js subcommand ([9dc010a](9dc010a))
* reset prettier and browserslists ([d1eab91](d1eab91))
* resolve cli-style path by using dirname ([3a61532](3a61532))
* update help text ([f820a66](f820a66))
* update pre-hook ([6988cf9](6988cf9))
* use reporter from helpers-engine ([ef79104](ef79104))
* use the new release script ([#31](#31)) ([2e2e454](2e2e454))
* use two-param version for pkg to identify assets ([284ce0b](284ce0b))

### Features

* add ability to pass in files to check and apply ([2e7fcdd](2e7fcdd))
* add editorconfig ([#13](#13)) ([8b0c392](8b0c392))
* add option to not stage formatted files ([00d5bc7](00d5bc7))
* allow check/apply commands for commit msgs ([aca257f](aca257f))
* convert to @dhis2/cli format ([d95179c](d95179c))
* set an exit code if js check fails ([#18](#18)) ([3833bea](3833bea))
* stop auto-install of prettier and other config files ([#29](#29)) ([8c030b1](8c030b1))

### BREAKING CHANGES

* removes the binaries for `code-style.js` and
`commit-style.js` and instead exposes a binary named `d2-style` which is
the subcommand which the `d2` cli will use.
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ismay ismay ismay approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@varl @ismay @dhis2-bot