Skip to content

Provide ormolu via direnv, run make formatc in github actions#1908

Merged
flokli merged 7 commits intodevelopfrom
ormolu-gh-actions
Nov 4, 2021
Merged

Provide ormolu via direnv, run make formatc in github actions#1908
flokli merged 7 commits intodevelopfrom
ormolu-gh-actions

Conversation

@flokli
Copy link
Contributor

@flokli flokli commented Nov 4, 2021

This will provide ormolu via direnv, and do some CI steps in GH Actions:

  • ensuring the direnv environment (still) builds
  • uploading it to the cachix cache
  • run make formatc and provide feedback if code is not formatted.

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • If HTTP endpoint paths have been added or renamed, the endpoint / config-flag checklist (see Wire-employee only backend wiki page) has been followed.
  • If a cassandra schema migration has been added, I ran make git-add-cassandra-schema to update the cassandra schema documentation.
  • changelog.d contains the following bits of information:
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.
    • If new config options introduced: added usage description under docs/reference/config-options.md
    • If new config options introduced: recommended measures to be taken by on-premise instance operators.
    • If a cassandra schema migration is backwards incompatible (see also these docs), measures to be taken by on-premise instance operators are explained.
    • If a data migration (not schema migration) introduced: measures to be taken by on-premise instance operators.
    • If public end-points have been changed or added: does nginz need un upgrade?
    • If internal end-points have been added or changed: which services have to be deployed in a specific order?

This builds the direnv, so it's available in the cachix binary cache.
This ensures the codebase is properly formatted.
@flokli flokli force-pushed the ormolu-gh-actions branch 3 times, most recently from 020e84a to 08b2023 Compare November 4, 2021 11:39
Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

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

Could you add a changelog line under changelog.d/5-internal/ ?

Copy link
Member

Choose a reason for hiding this comment

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

We already have a concourse check for the formatc thing; so these last two lines are not really necessary I'd say; but sure we can have the check run twice in case one CI is down.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Concourse CI Logs aren't visible externally, and in the past, they were red for quite some time without getting fixed. Having CI definitons in the same repo, and public readable logs allows everyone to send a PR to fix it.

It'd be pretty simple to require that gh actions pipeline to be green before merging. Adding a separate concourse pipeline only for that purpose would be much more involved.

Copy link
Member

Choose a reason for hiding this comment

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

The ormolu check from concourse is behaving just fine and hasn't had any problems I'm aware of. As for the "visible logs for externals", there is a believe an ongoing project for a concourse-on-k8s-for-CI from the PIT team that would solve that part, but not just for ormolu, but also for all the other checks that currently run on CI.

I already approved the PR so I'm not against adding this check a second time, it's just providing little extra value at this point IMO.

@flokli
Copy link
Contributor Author

flokli commented Nov 4, 2021

I added the changelog entry. PTAL.

@flokli flokli requested a review from jschaul November 4, 2021 14:38
Otherwise, ormolu fails with some locales on non-ASCII characters:

```
ormolu: libs/dns-util/src/Wire/Network/DNS/SRV.hs: hGetContents: invalid argument (invalid byte sequence)
```

See tweag/ormolu#38 and
https://gitlab.haskell.org/ghc/ghc/-/issues/17755 for details.
@flokli flokli force-pushed the ormolu-gh-actions branch from aca15a1 to e42d91a Compare November 4, 2021 14:43
@flokli flokli merged commit b0a3d60 into develop Nov 4, 2021
@flokli flokli deleted the ormolu-gh-actions branch November 4, 2021 19:49
@akshaymankar akshaymankar mentioned this pull request Nov 15, 2021
@smatting smatting mentioned this pull request Dec 2, 2021
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