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

Avoid duplicating YAML info in Markdown #5227

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

sellout
Copy link
Contributor

@sellout sellout commented Jul 15, 2024

Overview

Reviewing the last merge into #5142, I noticed some duplicated and out-of-date information. This brings things up-to-date and slightly reduces the duplication.

Implementation notes

  • bumped Ormolu used by ci.yaml from 0.5.2.0 to 0.7.2.0, to match flake.nix
  • removed Markdown that claimed ci.yaml was using Ormolu 0.5.0.1
  • moved description from base-codebase in Markdown to comment on runtime_tests_codebase in ci.yaml (and updated it to refer to builtin-tests/interpreter-tests.md instead of builtin-tests/base.md)
  • removed unison_src_test_results as it’s no longer managed as a single variable
  • moved other comments from Markdown to ci.yaml
  • added Markdown recommending to look in ci.yaml for specifics
  • rearranges the order of vars in ci.yaml to match the order they were presented in Markdown

Test coverage

This bumps the Ormolu version in ci.yaml, but otherwise just rearranges some items in that file. Running CI should catch if any of that is incorrect.

Loose ends

One thing I wasn’t sure how to map over: Markdown claims Racket 8.7 is used in CI, but ci.yaml doesn’t mention any Racket version.

Reviewing the last merge into unisonweb#5142, I noticed some duplicated and out-
of-date information. This brings things up-to-date and slightly reduces
the duplication.

- bumped Ormolu used by ci.yaml from 0.5.2.0 to 0.7.2.0, to match
  flake.nix
- removed Markdown that claimed ci.yaml was using Ormolu 0.5.0.1
- moved description from `base-codebase` in Markdown to comment on
 `runtime_tests_codebase` in ci.yaml (and updated it to refer to
   builtin-tests/interpreter-tests.md instead of builtin-tests/base.md)
- removed `unison_src_test_results` as it’s no longer managed as a
  single variable
- moved other comments from Markdown to ci.yaml
- added Markdown recommending to look in ci.yaml for specifics
- rearranges the order of vars in ci.yaml to match the order they were
  presented in Markdown

One thing I wasn’t sure how to map over: Markdown claims Racket 8.7 is
used in CI, but ci.yaml doesn’t mention any Racket version.
@sellout sellout marked this pull request as ready for review July 15, 2024 22:33
@sellout sellout requested a review from a team as a code owner July 15, 2024 22:33
@sellout sellout requested a review from aryairani July 15, 2024 22:33
@@ -12,19 +12,26 @@ on:
workflow_dispatch:

env:
ormolu_version: 0.5.2.0
ucm_local_bin: ucm-local-bin
## Some version numbers that are used during CI
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like some editor told me having comments in here was illegal, but I guess it's not true

@aryairani aryairani merged commit 525e923 into unisonweb:trunk Jul 16, 2024
20 checks passed
@sellout sellout deleted the deduplication branch August 15, 2024 05:50
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