Skip to content

Conversation

VitoAlbano
Copy link
Contributor

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation
  • Other... Please describe:

What is the current behaviour? (You can also link to an open issue here)

What is the new behaviour?

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@VitoAlbano VitoAlbano added do not merge 🙅🏻‍♂️ do not review 😢 Drama approaching This PR contains dramatic tears from the developers Coin to the witcher 🪙 Toss a cooooin to your witcheeeeeer labels Mar 13, 2025
@VitoAlbano VitoAlbano changed the title [AAE-32905] - Start by adding unit test matrix [AAE-32905] - Improving ADF PR workflow Mar 13, 2025
@VitoAlbano VitoAlbano force-pushed the improvement/AAE-32905-rework-PR-pipeline branch from 2c0a21d to 43887ff Compare March 29, 2025 21:55
@mauriziovitale
Copy link
Contributor

in my opinion it would be better to have the generate step for the specific target instead of 1 global

build
generete build matrix
build 1
build 2

unit
generete unit matrix
unit 1
unit 2

@VitoAlbano VitoAlbano force-pushed the improvement/AAE-32905-rework-PR-pipeline branch from 75cea0b to 8537138 Compare April 1, 2025 21:06
@@ -0,0 +1,52 @@
name: "Build Lib Workflow"
Copy link
Contributor

Choose a reason for hiding this comment

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

follow the naming convention
_build_libs.yaml

name: "Build Lib Workflow"

on:
workflow_call: {}
Copy link
Contributor

Choose a reason for hiding this comment

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

why do u need it? if there are no inputs

AFFECTED_UNIT=$(npx nx show projects --affected --target=test --base=origin/$BASE_REF --select=projects --plain --exclude=cli,stories,eslint-angular)
echo "Affected projects for BUILD : $AFFECTED_BUILD"
BUILD_MATRIX_JSON=$(echo $AFFECTED_BUILD | xargs -n1 | jq -R -s -c 'split("\n")[:-1] | map({ "project": . })')
BUILD_MATRIX_JSON=$(echo "$BUILD_MATRIX_JSON" | tr -d '\n' | sed 's/"$//')
Copy link
Contributor

Choose a reason for hiding this comment

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

why don t get json from the show instead of plain?

- name: Generate affected projects matrix
id: set-matrix
run: |
BASE_REF="${{ github.event.pull_request.base.ref }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

\i would prefer to have JS script or node action ( preferred)

@VitoAlbano VitoAlbano force-pushed the improvement/AAE-32905-rework-PR-pipeline branch from c32eec8 to e5295e5 Compare April 15, 2025 09:44
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Coin to the witcher 🪙 Toss a cooooin to your witcheeeeeer 😢 Drama approaching This PR contains dramatic tears from the developers do not merge 🙅🏻‍♂️ do not review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants