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.

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

add "mode" input #29

Closed
wants to merge 3 commits into from
Closed

Conversation

aryairani
Copy link
Contributor

Adding the mode input lets me change ormolu to inplace mode, which allows me to automatically format and commit my PRs instead of kicking them back to the contributor and making them figure out how to use ormolu 😅 .

It's split into multiple commits for cherry-picking.

@mrkkrp
Copy link
Collaborator

mrkkrp commented Jul 1, 2023

Wow, isn't it pretty annoying to have a commit history with every other message being "automated ormolu commit"?

@aryairani
Copy link
Contributor Author

aryairani commented Jul 2, 2023

To clarify, those "automated ormolu commit"s are not part of this PR, they're just part of an example that used it.

But to your question: It's not every other message, it's once per push, and even then only if the HEAD commit is improperly formatted and would otherwise fail CI and be kicked back to the contributor, who would manually run ormolu and commit the results with a message like "manual ormolu commit", which itself is better than the most common commit message, "wip". So it's not more annoying, no. 😅

@mrkkrp
Copy link
Collaborator

mrkkrp commented Jul 2, 2023

and commit the results with a message like "manual ormolu commit"

I'd just squash the changes and never have any re-formatting commits.

@mrkkrp
Copy link
Collaborator

mrkkrp commented Jul 2, 2023

But I guess it doesn't hurt to have an extra setting for people who want this kind of workflow. I will return to this in the coming week and take another look, then merge.

@mrkkrp
Copy link
Collaborator

mrkkrp commented Jul 4, 2023

Applied as 46243e8.

@mrkkrp mrkkrp closed this Jul 4, 2023
@mrkkrp
Copy link
Collaborator

mrkkrp commented Jul 4, 2023

Would you like me to make a new release with this feature? Is it urgent for you?

@aryairani
Copy link
Contributor Author

@mrkkrp I'm using it already from my fork, so it's not urgent; but once it's released I'll switch back to using the original repo :)

@mrkkrp
Copy link
Collaborator

mrkkrp commented Jul 5, 2023

v14 is now available :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants