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

Fixes #1272 #1372

Merged
merged 1 commit into from
Oct 19, 2023
Merged

Fixes #1272 #1372

merged 1 commit into from
Oct 19, 2023

Conversation

BubuDavid
Copy link
Contributor

Description of Changes

Closes #1272


New Version Info

Author's Instructions

  • Derive a new MAJOR.MINOR.PATCH version number. Increment the:
    • MAJOR version when you make incompatible API changes
    • MINOR version when you add functionality in a backwards-compatible manner
    • PATCH version when you make backwards-compatible bug fixes
  • Update CHANGELOG.md, following the established pattern.

Collaborator's Instructions

  • Review CHANGELOG.md, suggesting a different version number if necessary.
  • After merging, tag the commit using these (Mac-compatible) bash commands:
    git checkout master
    git pull
    sed -n "$(grep -n -m2 '####' CHANGELOG.md | cut -f1 -d: | sed 'N;s/\n/,/')p" CHANGELOG.md | sed '$d'
    git tag -a $(read -p "Tag Name: " tag;echo $tag) -m"$(git show --quiet --pretty=%s)";git push origin --tags

Copy link
Member

@rzvxa rzvxa left a comment

Choose a reason for hiding this comment

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

@BubuDavid Thanks for the change
@alerque Would you mind reviewing this PR?

@rzvxa
Copy link
Member

rzvxa commented Oct 19, 2023

Hello @BubuDavid,

I would like to go ahead and merge this PR. Can you rebase your work to the current version of the master?
If you don't have time to do so Or have lost interest in doing so I can do it for you, If that is the case just let me know.
I'm sorry it took a while before merging it.

@alerque
Copy link
Member

alerque commented Oct 19, 2023

@rzvxa This doesn't need rebasing, you can merge with the "rebase and merge" option to get linear history while keeping the commits from the PR intact or "squash and merge" if the PR has messy commit history but is overall a small single-purpose set of changes. You only really need to rebase inside a PR if there are going to be merge conflicts or some testing/review is proving hard to do on an old branch point.

@rzvxa
Copy link
Member

rzvxa commented Oct 19, 2023

@alerque I couldn't access the rebase and merge because of the fork being out of date, Should I use merge commits for updating them if I do want to keep the original commits intact? Because rebase and update changed the original commits

@rzvxa
Copy link
Member

rzvxa commented Oct 19, 2023

Since this one is already rebased and also original author is kept intact I'm going to move forward with merging it, But I want to know if the right way is doing the merge commit instead. I don't like it when commits get changed, And I wasn't aware of the fact that GitHub doesn't respect the original commit author information, Since most of my work is in a closed source environment it wasn't an issue for me until now. I would love it if you could tell me how to update a branch in GitHub PR without changing the commits and ideally without any additional merge commit on top, Until I find out more about it I'm going to use merge commit since it won't rewrite the commits in the fork even tho I'm not a big fan of merge commits for small changes like this. (For more complex changes it makes sense to me since you get merge conflicts, But a one-file change where it's only behind by a commit is pretty obvious that won't have any conflicts)
Pardon me if I got it wrong. I'm used to thinking about commits as diffs, So in my mind, it won't affect the overall outcome if you apply a diff before or after another one when they are changing different files.

@rzvxa rzvxa merged commit d69b68b into preservim:master Oct 19, 2023
1 check 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.

Nerdtree window switches/swaps with main window, when opened with directory as argument
3 participants