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

github: Enhance linters to warn if branch is not rebased when errors occur #4255

Conversation

OhmSpectator
Copy link
Member

This pull request enhances both the commit message and SPDX linters by adding a check to determine if the current branch is rebased on top of the base branch when errors are detected. If issues are found and the branch is not rebased, the linters inform the user that the problems may be due to the branch not being up-to-date. This provides more informative feedback in the GitHub Action logs when the linters fail, helping developers diagnose and resolve issues more efficiently.

Add `check_branch_rebased` to verify if the current branch is rebased on
`BASE_COMMIT`. This helps identify if issues are due to the branch being
outdated.

Signed-off-by: Nikolay Martyanov <nikolay@zededa.com>
Add `check_branch_rebased` to verify if the current branch is rebased on
`base_hash. If not, inform the user that errors may be due to the branch
not being rebased.

Signed-off-by: Nikolay Martyanov <nikolay@zededa.com>
@OhmSpectator OhmSpectator force-pushed the feature/linters-to-show-warning-if-not-rebased branch from 19c914a to 92351de Compare September 13, 2024 16:54
Copy link
Contributor

@deitch deitch left a comment

Choose a reason for hiding this comment

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

Yes, please. I fell prey to this twice in the last day. "Why is it complaining about that file, when I didn't touch it?"

Would it be better to learn to ignore those files, or is that too hard to do? Let's say our commit tree is:

A -> B -> C (master)
          |-> D (mybranch)

What you are fixing is, that something might have changed B-C, so the checker complains about it, even though D didn't affect that file.

While I agree with rebasing, it isn't always necessary. Would it be possible for this instead to just ignore those files that are unchanged between B->D, rather than saying, "you are not based on C, so go rebase on C, and then I will check C->D diff for you"?

@OhmSpectator
Copy link
Member Author

Yes, please. I fell prey to this twice in the last day. "Why is it complaining about that file, when I didn't touch it?"

Would it be better to learn to ignore those files, or is that too hard to do? Let's say our commit tree is:

A -> B -> C (master)
          |-> D (mybranch)

What you are fixing is, that something might have changed B-C, so the checker complains about it, even though D didn't affect that file.

While I agree with rebasing, it isn't always necessary. Would it be possible for this instead to just ignore those files that are unchanged between B->D, rather than saying, "you are not based on C, so go rebase on C, and then I will check C->D diff for you"?

I think it should be possible. But will require some work and testing. I believe that next week, I will have fun with some urgent bug fixes and cannot propose a better approach for this issue. So I would suggest taking this and later I'll return to a better approach.

@deitch
Copy link
Contributor

deitch commented Sep 15, 2024

Ok that's fair enough. This is an improvement on its own. Let's accept this and then iterate further.

@OhmSpectator OhmSpectator merged commit 5b878f3 into lf-edge:master Sep 16, 2024
46 of 48 checks passed
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