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

Jasmeen/update bom logo 727 #738

Merged
merged 1 commit into from
Aug 28, 2024
Merged

Conversation

KAUR1984
Copy link
Contributor

Fixes the issue #727.

@KAUR1984 KAUR1984 added the ✨UI Improvements or additions to the UI label Aug 27, 2024
@KAUR1984 KAUR1984 requested a review from a team August 27, 2024 02:23
@KAUR1984 KAUR1984 self-assigned this Aug 27, 2024
Copy link

github-actions bot commented Aug 27, 2024

PR preview
⚠️ There was an error in the pr-preview deployment. For more information, please check the Actions tab.
2024-08-28 15:54 AEST

Copy link
Contributor

@atteggiani atteggiani left a comment

Choose a reason for hiding this comment

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

The changes are fine, but the branch contains commits that are not part of this PR.

Please clean the branch keeping only the commits that are relevant (I think only 618d924de40ec09c1d19bc1df4600b4c21259944)

@KAUR1984
Copy link
Contributor Author

Thanks @atteggiani ! I haven't tweaked the history of branch commits. I did some pulls (from development and main) and pushes, and that's what it seems to be reflecting :)

@atteggiani
Copy link
Contributor

atteggiani commented Aug 27, 2024

Thanks @atteggiani ! I haven't tweaked the history of branch commits. I did some pulls (from development and main) and pushes, and that's what it seems to be reflecting :)

I see, the branch only needs to be rebased to development.

@KAUR1984
Copy link
Contributor Author

KAUR1984 commented Aug 27, 2024

Thanks Davide! I really like the non-destructive way of handling commits - using git merge, would not use git rebase unless I absolutely have to. Very happy to discuss it a bit more in our discussion :) Happy to leave this PR open till then :)

@atteggiani
Copy link
Contributor

atteggiani commented Aug 27, 2024

Thanks Davide! I really like the non-destructive way of handling commits - using git merge, would not use git rebase unless I absolutely have to. Very happy to discuss it a bit more in our discussion :) Happy to leave this PR open till then :)

I am not talking about the way (merge/rebase) to add commits from feature branches (like your jasmeen/update-bom-logo-727 to the development branch. We've been using merge-commits for that and is completely fine.

The jasmeen/update-bom-logo-727 is not up to date with development.
This means that there are commits in developments that were added after jasmeen/update-bom-logo-727 was created (the commits are not included in the history of jasmeen/update-bom-logo-727).
In general, it is always better to rebase the feature branches with development before merging to it (with a merge-commit), so we avoid any conflicts and make sure we are on track with development. In this case we should rebase and not merge because we don't want an additional merge-commit to be created for commits that are already present inside development.

Hopefully this explanation made sense.
Please let me know if you need further clarification :)

@KAUR1984 KAUR1984 merged commit 854dbdb into development Aug 28, 2024
2 of 4 checks passed
@KAUR1984 KAUR1984 deleted the jasmeen/update-bom-logo-727 branch August 28, 2024 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨UI Improvements or additions to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants