implement reusable workflows for module integration testing #75
Conversation
|
The created documentation from the pull request is available at: docu-html |
| required: false | ||
| type: string | ||
| default: 'bl-x86_64-linux' | ||
| ref_int_ref: |
There was a problem hiding this comment.
I don't get these two: ref_int_ref and 'ref_int_repo`
There was a problem hiding this comment.
ref = commit/branch/tag ?!
There was a problem hiding this comment.
Maybe it's target_branch_in_integration?
There was a problem hiding this comment.
This is about referencing the ref_int repo. Mainly I need ref for the checkout later. The repo is to enable users to use this workflow also in a private clone.
ref_int_ref = commit/branch/tag
ref_int_repo = eclipse-score/reference_integration (or your private org)
|
LGTM, but it wouldn't hurt to add some documentation about the usage. Maybe add a section in the README? |
- Refactor Python tooling to use shared Module dataclass
* Created tools/models/module.py with centralized module schema
* Added methods: from_dict, parse_modules, owner_repo, to_dict
* Migrated scripts to use dataclass: update_module_latest.py,
update_module_from_known_good.py, override_known_good_repo.py,
known_good_to_workspace_metadata.py, get_module_info.py
* Eliminates duplicate parsing logic across tools
- Clean up override_known_good_repo.py
* Renamed --repo-override to --module-override
* Removed backward compatibility for deprecated flags
* Extracted write_known_good() function for better reusability
* Updated examples to reflect supported formats only
- Improve workflow documentation
* Clarified ref_int_repo and ref_int_ref semantics in smoke-test workflow
* Added detailed usage notes for private fork support
* Updated workflow steps to use --module-override flag
* Enhanced input descriptions for runner label selection
- Remove generated artifact from git
* Deleted known_good.updated.json (build output)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
tools/update_module_latest.py:102
- Inconsistency in branch serialization. In
write_known_good, the branch is always written if present, but inModule.to_dict()(line 108 of module.py), the branch is only written if it differs from 'main'. This inconsistency can lead to the branch field being included unnecessarily in the output, conflicting with the Module class's design. Consider usingm.to_dict()or only writing branch when it's not 'main'.
if m.branch:
mod_dict["branch"] = m.branch
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Output the content as a JSON string | ||
| echo "known_good_updated<<EOF" >> $GITHUB_OUTPUT | ||
| cat known_good.updated.json >> $GITHUB_OUTPUT | ||
| echo "EOF" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
Using multiline output for potentially large JSON content could hit GitHub Actions output size limits for large known_good.json files. Consider using artifacts exclusively for passing this data between jobs instead of job outputs, as artifacts are already being uploaded at line 120-123.
There was a problem hiding this comment.
I can live with both solutions. @AlexanderLanin what is your opinion?
There was a problem hiding this comment.
Artifacts between actions is most likely best as that could also enable you (depending on how it's set up) to download them and see what is going wrong.
At least I think that is an option then.
| required: false | ||
| type: string | ||
| default: 'bl-x86_64-linux' | ||
| repo_runner_labels: |
There was a problem hiding this comment.
is the runner provided by the caller, or would it be better here in env?
| # "[\"self-hosted\",\"linux\",\"x64\"]"). | ||
| # - target_branch (string, optional, default: main): Ref/branch to checkout. | ||
| # | ||
| # Repository Variables: |
There was a problem hiding this comment.
any idea from which repo this is used? The caller, this one, or reference_integration?
AlexanderLanin
deleted the
feature/1852-prepare-script-for-integration-feedback-on-PR
branch
4 participants
resolves eclipse-score/score#1852