Skip to content

Consolidate import export comma styles#207

Merged
brandonchinn178 merged 5 commits intofourmolu:mainfrom
3kyro:201-consolidate-import-export-comma-styles
Jun 20, 2022
Merged

Consolidate import export comma styles#207
brandonchinn178 merged 5 commits intofourmolu:mainfrom
3kyro:201-consolidate-import-export-comma-styles

Conversation

@3kyro
Copy link
Copy Markdown
Contributor

@3kyro 3kyro commented Jun 19, 2022 •
edited
Loading

Consolidate import-export-comma-style and diff-friendly-import-export into a new option import-export-style as per #201

cc @brandonchinn178

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jun 19, 2022 •
edited by brandonchinn178
Loading

👋 @3kyro
Thank you for raising your pull request.
Please make sure you have followed our contributing guidelines. We will review it as soon as possible!

Reviewer: Please verify the following things have been done, if applicable.

  • CHANGELOG.md has been updated
  • Configuration docs in README.md have been updated
  • fourmolu.yaml updated to stay in sync with config in README.md
  • Tests have been added

brandonchinn178
brandonchinn178 reviewed Jun 19, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@brandonchinn178 brandonchinn178 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! A few comments; can you also make sure to update the "Specifying config" section in the README too?

brandonchinn178
brandonchinn178 reviewed Jun 20, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@brandonchinn178 brandonchinn178 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're still missing the README change I asked about, in the "Specifying configuration" section

@3kyro
Copy link
Copy Markdown
Contributor Author

3kyro commented Jun 20, 2022

I think you're still missing the README change I asked about, in the "Specifying configuration" section

ah, yes I did :(. I'm not sure why the fourmolu style was mentioning leading commas, but default configuration should be diff-friendly.

brandonchinn178
brandonchinn178 approved these changes Jun 20, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@brandonchinn178 brandonchinn178 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you!

@brandonchinn178 brandonchinn178 merged commit ef92a63 into fourmolu:main Jun 20, 2022
@georgefst georgefst mentioned this pull request Jun 20, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brandonchinn178 brandonchinn178 brandonchinn178 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@3kyro @brandonchinn178