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

PLT-42 Added cabal-fmt to pre-commit hooks & reformatted all existing… #505

Merged
merged 29 commits into from
Jun 15, 2022
Merged

PLT-42 Added cabal-fmt to pre-commit hooks & reformatted all existing… #505

merged 29 commits into from
Jun 15, 2022

Conversation

zeme-iohk
Copy link

cabal-fmt is now available inside nix-shell, and as a pre-commit hook.
I have personally tested it only on one cabal file, to keep this PR small, and so that you may have an idea of what the new format looks like.
As soon as a change is made to any other cabal file from anybody else, the tool will format it.
But I'm open to format all other cabal files and add a new commit to this PR.

raduom
raduom reviewed Jun 9, 2022
View reviewed changes
Copy link
Contributor

@raduom raduom left a comment

Choose a reason for hiding this comment

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

Do we need to change anything in CONTRIBUTING or any other development docs?

zeme-iohk
zeme-iohk commented Jun 9, 2022
View reviewed changes
shell.nix Show resolved Hide resolved
@zeme-iohk
Copy link
Author

Do we need to change anything in CONTRIBUTING or any other development docs?

I updated that in a new commit.

@koslambrou
Copy link
Contributor

Additional things would be:

  • There should be a command format ALL cabal files (something called fix-cabal) which works in a similar way to fix-stylish-haskell.
  • Also, like fix-stylish-haskell, there should also be a check on CI is someone forgot to format the cabal files

koslambrou
koslambrou reviewed Jun 9, 2022
View reviewed changes
CONTRIBUTING.adoc Outdated Show resolved Hide resolved
michaelpj
michaelpj approved these changes Jun 9, 2022
View reviewed changes
Copy link
Contributor

@michaelpj michaelpj 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 should format all the cabal files. Otherwise the will randomly get formatted in other people's PRs, which is confusing for them. Better to move to the new desired state in one go.

@koslambrou
Copy link
Contributor

Also, rebase your branch on the latest commit of main to get the fix for the failing CI.

@michaelpj
Copy link
Contributor

Still needs a test for CI.

koslambrou
koslambrou requested changes Jun 10, 2022
View reviewed changes
plutus-contract/plutus-contract.cabal Show resolved Hide resolved
plutus-contract/plutus-contract.cabal Outdated Show resolved Hide resolved
plutus-playground-server/plutus-playground-server.cabal Outdated Show resolved Hide resolved
plutus-playground-server/plutus-playground-server.cabal Outdated Show resolved Hide resolved
plutus-use-cases/plutus-use-cases.cabal Outdated Show resolved Hide resolved
plutus-use-cases/plutus-use-cases.cabal Outdated Show resolved Hide resolved
koslambrou
koslambrou requested changes Jun 10, 2022
View reviewed changes
plutus-use-cases/plutus-use-cases.cabal Show resolved Hide resolved
default.nix Outdated Show resolved Hide resolved
@zeme-iohk zeme-iohk force-pushed the zeme-iohk/PLT-42-plutus-apps-add-a-cabal-files-formatter-2 branch from 5b00ff6 to 4d6b246 Compare June 14, 2022 13:11
@zeme-iohk
Copy link
Author

This branch appears to contain a bunch of extraneous commits?

This was a rebase gone wrong. It's now fixed.

koslambrou
koslambrou reviewed Jun 14, 2022
View reviewed changes
web-ghc/web-ghc.cabal Outdated Show resolved Hide resolved
@michaelpj
Copy link
Contributor

FWIW, I think this PR would be smoother if we left the actual content changes (e.g. fixing the plutus/plutus-apps stuff) to a later PR and kept this one just focussed on formatting.

@koslambrou
Copy link
Contributor

koslambrou commented Jun 14, 2022 •
edited
Loading

@michaelpj Right :) The formatting looks good, so I think we can merge once CI passes.

@zeme-iohk zeme-iohk merged commit b307823 into main Jun 15, 2022
@zeme-iohk zeme-iohk deleted the zeme-iohk/PLT-42-plutus-apps-add-a-cabal-files-formatter-2 branch June 15, 2022 09:02
koslambrou pushed a commit that referenced this pull request Jun 22, 2022
@zeme-wana @koslambrou
PLT-42 Added cabal-fmt to pre-commit hooks & reformatted all existing… (
caa5014 
#505)

* PLT-42 Added cabal-fmt to pre-commit hooks & reformatted all existing cabal files
* Created fix-cabal-fmt script and added to shell
* Added CI test for cabal-fmt
* Updated CONTRIBUTING.adoc to include addition of cabal-fmt
* Replaced occurences of iohk/plutus with input-output-hk/plutus-apps
koslambrou pushed a commit that referenced this pull request Apr 6, 2023
@zeme-wana
PLT-42 Added cabal-fmt to pre-commit hooks & reformatted all existing… (
397074c 
#505)

* PLT-42 Added cabal-fmt to pre-commit hooks & reformatted all existing cabal files
* Created fix-cabal-fmt script and added to shell
* Added CI test for cabal-fmt
* Updated CONTRIBUTING.adoc to include addition of cabal-fmt
* Replaced occurences of iohk/plutus with input-output-hk/plutus-apps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjoerdvisscher sjoerdvisscher sjoerdvisscher left review comments

@michaelpj michaelpj michaelpj approved these changes

@raduom raduom raduom left review comments

@koslambrou koslambrou koslambrou approved these changes

@joseph-fajen joseph-fajen Awaiting requested review from joseph-fajen

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

Successfully merging this pull request may close these issues.
6 participants
@zeme-iohk @koslambrou @michaelpj @sjoerdvisscher @raduom @zeme-wana