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 west patch subcommand #59998

Closed
wants to merge 1 commit into from

Conversation

mbrunnen
Copy link
Contributor

@mbrunnen mbrunnen commented Jul 4, 2023

west patch: E.g. this allows to stay on a mainline zephyr revision, while
maintaining patch files in the project SDK. This can solve the "two pace"
problem for long zephyr release cycles versus immediate project need,
without maintaining a fork.

`west patch`: E.g. this allows to stay on a mainline zephyr revision, while
maintaining patch files in the project SDK. This can solve the "two pace"
problem for long zephyr release cycles versus immediate project need,
without maintaining a fork.

Signed-off-by: Manoel Brunnen <[email protected]>
@nordicjm
Copy link
Collaborator

nordicjm commented Jul 4, 2023

E.g. this allows to stay on a mainline zephyr revision, while
maintaining patch files in the project SDK

So when people post issues and give a commit ID for the version with the issue, this is now likely to be invalid because they could have applied random patches? Not sure I see the point in this

@mbrunnen
Copy link
Contributor Author

mbrunnen commented Jul 4, 2023

Which would be also true when maintaining a project fork.

@nordicjm
Copy link
Collaborator

nordicjm commented Jul 4, 2023

If you're working on a fork, why would you need this when you can branch and push as much as you want?

@mbrunnen
Copy link
Contributor Author

mbrunnen commented Jul 4, 2023

In our projects we need our upstreamed patches, but we want to stay on a release revision for zephyr and possibly other modules. So we keep patch files in our SDK and apply them with west patch. We keep them until these patches have been integrated in a release.

Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

Hi there. Thanks for the proposal. I am nacking this because:

  1. This PR and command belong in the west repo as a built-in command, not an extension one
  2. This has been discussed here: Add git-patch feature in west update west#596

@mbrunnen
Copy link
Contributor Author

mbrunnen commented Jul 4, 2023

If you're working on a fork, why would you need this when you can branch and push as much as you want?

Sorry for the misunderstanding, maintaining forks is exactly what we try to avoid.

Copy link
Collaborator

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

Sorry for the misunderstanding, maintaining forks is exactly what we try to avoid.

But you are maintaining a fork, albeit one that is worse than if you just forked the repository and hosted that instead whereby you'd be able to do things like compare git commit IDs to see what changes were introduced or be able to rebase, for that reason I am NACK'ing this.

If you want to use this, you should have it as part of your own custom west commands in, let's say, a forked zephyr repository

@mbrunnen
Copy link
Contributor Author

mbrunnen commented Jul 5, 2023

I am using this, in our SDK module. git am does normal commits, I can use the git tooling like in a normal branch.

@mbrunnen mbrunnen closed this Jul 5, 2023
@mbrunnen mbrunnen deleted the west-patch branch July 5, 2023 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: West West utility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants