Skip to content

Replacing the download scripts with West - #85

Draft
bignaux wants to merge 1 commit into
ps2homebrew:wOPL-basefrom
bignaux:west
Draft

bignaux wants to merge 1 commit into
ps2homebrew:wOPL-basefrom
bignaux:west

Conversation

@bignaux

@bignaux bignaux commented Apr 3, 2026

Copy link
Copy Markdown
Member

Replacing the download scripts with West.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request replaces custom shell scripts for dependency management with the west tool, updating the Makefile and requirements.txt while removing the obsolete scripts. Feedback indicates that the transition is currently incomplete as it lacks a west.yml manifest and workspace initialization logic, which will cause build failures. Additionally, it is recommended to pin the version of west in requirements.txt to ensure build reproducibility.

Comment thread Makefile
Comment on lines +849 to +855
west update lng_src

download_lwNBD:
sh download_lwNBD.sh
west update lwNBD

download_cfla:
sh download_cfla.sh
west update clang-format-lint-action

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

The transition to west is incomplete and will break the build process. west update requires a west.yml manifest file and an initialized workspace (a .west directory). This PR removes the existing download scripts but does not provide the necessary west.yml file. Additionally, for a seamless developer experience, the Makefile should handle workspace initialization (west init -l .) if it hasn't been performed yet, as the previous scripts handled the cloning logic automatically.

	[ -d .west ] || west init -l .
	west update lng_src

download_lwNBD:
	[ -d .west ] || west init -l .
	west update lwNBD

download_cfla:
	[ -d .west ] || west init -l .
	west update clang-format-lint-action

Comment thread requirements.txt
@@ -1 +1,2 @@
PyYAML==6.0
west

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Pinning the version of west is recommended to ensure build reproducibility across different environments and to prevent potential breakages from future updates to the tool.

west==1.2.0
References
  1. Pinning dependency versions ensures build reproducibility and prevents unexpected breakages from upstream updates.

@bignaux
bignaux marked this pull request as draft April 3, 2026 20:40
@Wolf3s
Wolf3s force-pushed the wOPL-base branch 2 times, most recently from 479cd28 to 8e3a5a1 Compare April 9, 2026 10:22
@codacy-production

codacy-production Bot commented May 24, 2026

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 1 minor

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category Results
CodeStyle 1 minor

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric Results
Complexity 0
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

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.

1 participant