atomist-skills / prettier-skill

Atomist Skill to format your code using Prettier
Apache License 2.0
0 stars 0 forks source link

Consider updating description of Fix problems parameter #71

Closed ddgenome closed 4 years ago

ddgenome commented 4 years ago

The description of the Fix problems parameter:

Run Prettier with --write option and determine how and when fixes should be committed back into the repository

Led me to think it only applied to fixes in formatting created by prettier. It turns out it also applies to the changes made when updating prettier configuration and git hooks.

I'd recommend updating the description of the parameter or separating the behavior of prettier fixes and prettier configuration/hook updates.

ddgenome commented 4 years ago

And when you tell it to commit directly to the branch, it still seems to open a PR for config changes.

cdupuis commented 4 years ago

@ddgenome there's an entirely different section that controls the update config behaviour at the bottom of the config page. That's that part that's responsible for the updates. Did you use that?

image
ddgenome commented 4 years ago

Yes, I had that set to the value you show in your screenshot, but no changes were made to the config. For example

https://go.atomist.com/T095SFFBK/log/ad47380b-2254-4e8b-99a3-5fdaa1e481da/7AwncTXwEf9oPG2To5Rb8?skill-filter=%5B%7B%22name%22:%22prettier-skill%22,%22namespace%22:%22atomist%22%7D%5D

Looking at the code, it appears changes are only pushed if the "Fix problems" setting is not "none".

https://github.com/atomist-skills/prettier-skill/blob/4ac399343bfe661a4804da0036eb25e34e04d3fb/lib/events/updateToolsOnPush.ts#L261-L264

Perhaps that should key off the configuration parameter, https://github.com/atomist-skills/prettier-skill/blob/4ac399343bfe661a4804da0036eb25e34e04d3fb/lib/configuration.ts#L31 .

cdupuis commented 4 years ago

Yeah, that first code snippet is a bug. Which btw. this bug is the same in the eslint-skill :-(