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

Add support for specifying the workspace manifest file name #9

Merged
merged 1 commit into from
Mar 19, 2024
Merged

Conversation

sense-Jo
Copy link
Contributor

West supports specifying the name of the workspace manifest file name when initializing a workspace. A use case for this would be a Zephyr application which can be build with vanilla Zephyr and nRF Connect SDK.

Copy link
Member

@fabiobaltieri fabiobaltieri left a comment

Choose a reason for hiding this comment

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

Thanks! Just a minor comment.

action.yml Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
Copy link
Member

@fabiobaltieri fabiobaltieri left a comment

Choose a reason for hiding this comment

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

Actually, can you squash the fix into the first commit? (and force push) Realized now you made two separate commits.

Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

Since this repo has a nice README with example scenarios, may I suggest you add one entry illustrating how to use the manifest-file-name?

Note: if this is to be released in a future version of the action (@v2?), the samples in the README will need updating at some point to use this new version identifier.

@fabiobaltieri
Copy link
Member

fabiobaltieri commented Mar 18, 2024

Note: if this is to be released in a future version of the action (@v2?), the samples in the README will need updating at some point to use this new version identifier.

Since the change is backward compatible I would just tag it as v1.0.2 and update v1 to point to it, I think that's how actions are supposed to be maintained, v2 would be for a potentially breaking change, so no need to update the samples for now.

@sense-Jo
Copy link
Contributor Author

sense-Jo commented Mar 19, 2024

Since this repo has a nice README with example scenarios, may I suggest you add one entry illustrating how to use the manifest-file-name?

Just added a commit with this.

@kartben
Copy link
Collaborator

kartben commented Mar 19, 2024

Since this repo has a nice README with example scenarios, may I suggest you add one entry illustrating how to use the manifest-file-name?

Just added a commit with this.

There is a minor typo ("REAMDE") in the commit message, but looks good otherwise, thanks! Might even make sense to just squash everything into just one commit TBH

Includes an additional scenario illustrating a custom manifest file name
in the README file.
@sense-Jo
Copy link
Contributor Author

Might even make sense to just squash everything into just one commit TBH

Alright, just did this.

@fabiobaltieri fabiobaltieri merged commit 5be4bd1 into zephyrproject-rtos:main Mar 19, 2024
4 checks passed
@fabiobaltieri
Copy link
Member

fabiobaltieri commented Mar 19, 2024

@sense-Jo just pushed the v1.0.2 tag past this and updated v1 to point to it as well, so this is now live.

Cheers.

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.

3 participants