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

CI - Reuse organisation's workflows to lint, test, tag and publish #160

Merged
merged 6 commits into from
Nov 2, 2021

Conversation

MrChocolatine
Copy link
Member

@MrChocolatine MrChocolatine commented Oct 29, 2021

Build

Update @embroider/test-setup to v0.47.1 (#160)

Disable Embroider's compatibility adapter for ember-get-config (#160)

See the dedicated commit for extensive details about these changes.

Deduplicate packages (#160)

See https://github.com/atlassian/yarn-deduplicate.

CI

Reuse organisation's workflow to run lint & tests (#160)

This change also activates the tests for Embroider.

Reuse organisation's workflow to run tag & publish (#160)

@MrChocolatine MrChocolatine marked this pull request as ready for review October 29, 2021 13:25
@MrChocolatine MrChocolatine force-pushed the ci-revamp-gh-actions branch 14 times, most recently from bc190f7 to d1fe4aa Compare October 29, 2021 14:21
EMBER_TRY_SCENARIO: ${{ matrix.ember-try-scenario }}
run: node_modules/.bin/ember try:one $EMBER_TRY_SCENARIO
org:
uses: peopledoc/.github/.github/workflows/emberjs-addons-ci.yml@add-reusable-workflow-emberjs-addon
Copy link
Member Author

@MrChocolatine MrChocolatine Oct 29, 2021

Choose a reason for hiding this comment

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

A version (here @add-reusable-workflow-emberjs-addon) is required otherwise we get the error:

https://github.com/peopledoc/ember-feature-controls/actions/runs/1399127533

invalid value workflow reference: no version specified

Comment on lines +10 to +13
concurrency:
group: ci-${{ github.head_ref || github.ref }}
cancel-in-progress: true
Copy link
Member Author

@MrChocolatine MrChocolatine Oct 29, 2021

Choose a reason for hiding this comment

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

This part cannot be moved in the reusable workflow (not supported yet), otherwise we get the error:

https://github.com/peopledoc/ember-feature-controls/actions/runs/1399155003

The workflow is not valid. peopledoc/ember-feature-controls/.github/workflows/org-ci.yml@ci-revamp-gh-actions (Line: 17, Col: 1): Concurrency is not yet supported at the root of a called workflow file

It must be declared in the caller workflow.


But it will be possible very soon, once this feature moves out of beta!

https://docs.github.com/en/actions/learn-github-actions/reusing-workflows#limitations

The following limitations will be removed when workflow reuse moves out of beta:
You can't set the concurrency of a called workflow from the caller workflow.

@MrChocolatine MrChocolatine requested a review from a team October 29, 2021 14:59
@MrChocolatine MrChocolatine changed the title ci: test reusable workflows CI - Reuse organisation workflow to run lint and tests Oct 29, 2021
@MrChocolatine MrChocolatine marked this pull request as draft October 29, 2021 15:05
@MrChocolatine MrChocolatine force-pushed the ci-revamp-gh-actions branch 8 times, most recently from ab77014 to 181d387 Compare November 1, 2021 13:18
@MrChocolatine MrChocolatine marked this pull request as ready for review November 1, 2021 13:19
@MrChocolatine MrChocolatine marked this pull request as draft November 1, 2021 13:33
@MrChocolatine MrChocolatine changed the title CI - Reuse organisation workflow to run lint and tests CI - Reuse organisation's workflows to lint, test, tag and publish Nov 1, 2021
@MrChocolatine MrChocolatine force-pushed the ci-revamp-gh-actions branch 2 times, most recently from 7dfc022 to 88dd395 Compare November 1, 2021 15:09
@MrChocolatine MrChocolatine added the dependencies Pull requests that update a dependency file label Nov 1, 2021
@MrChocolatine MrChocolatine force-pushed the ci-revamp-gh-actions branch 4 times, most recently from 9b3a3ed to e944315 Compare November 1, 2021 19:17
@MrChocolatine MrChocolatine force-pushed the ci-revamp-gh-actions branch 5 times, most recently from ce257ad to 962fa9b Compare November 2, 2021 12:35
`ember-get-config`, latest version 0.5.0, doesn't support Embroider yet
as this PR has not been merged as the time of writing this commit:

mansona/ember-get-config#29

In parallel, Embroider tests scenarios fail on our side:

https://github.com/peopledoc/ember-feature-controls/actions/runs/1408588177
  |
  `--> Relates to this issue:
       embroider-build/embroider#823

This issue 823 recommends to use `require('@embroider/compat').compatBuild`
in order to disable Embroider's adapter `ember-get-config` (see also
https://github.com/embroider-build/embroider/tree/v0.47.1#options).

But it does not work: `app` is not accepted by `compatBuild()` because
it is an `EmberAddon`, not an `EmberApp`. Log from the error:

```
Argument of type 'EmberAddon' is not assignable to parameter of type 'EmberAppInstance'.
  Property 'testIndex' is missing in type 'EmberAddon' but required in type 'EmberAppInstance'.ts(2345)
```

However, the following document indicates we can pass these options to
`maybeEmbroider()` to configure Embroider:

https://github.com/embroider-build/embroider/tree/v0.47.1/packages/test-setup#maybeembroiderapp-embroideroptions

References:
- Embroider's adapter for `ember-get-config`:
  https://github.com/embroider-build/embroider/blob/v0.47.1/packages/compat/src/compat-adapters/ember-get-config.ts

- Embroider compatibility options:
  https://github.com/embroider-build/embroider/blob/v0.47.1/packages/compat/src/options.ts#L46-L61

- Discussions on Discord where I first saw the same initial issue:
  - https://discordapp.com/channels/480462759797063690/568935504288940056/901170716949512233
  - https://discordapp.com/channels/480462759797063690/568935504288940056/902484167915364433
@MrChocolatine MrChocolatine marked this pull request as ready for review November 2, 2021 15:47
@MrChocolatine MrChocolatine merged commit 16a66dc into master Nov 2, 2021
@MrChocolatine MrChocolatine deleted the ci-revamp-gh-actions branch November 2, 2021 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants