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

Call vdeprecationSupplemental from deprecationSupplemental #7169

Merged
merged 1 commit into from
Sep 27, 2017

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Sep 26, 2017

Seems like it should do this to be consistent with errorSupplemental and warningSupplemental.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@mathias-lang-sociomantic
Copy link
Contributor

You have to change some tests it seems

@ibuclaw
Copy link
Member Author

ibuclaw commented Sep 26, 2017

I thought that might be the case.

@ibuclaw
Copy link
Member Author

ibuclaw commented Sep 27, 2017

@mathias-lang-sociomantic - you seem to have merged the commit in a weird way.

@mathias-lang-sociomantic
Copy link
Contributor

How weird ? It was a Rebase & Merge

@ibuclaw
Copy link
Member Author

ibuclaw commented Sep 28, 2017

There is no merge commit.

@ibuclaw
Copy link
Member Author

ibuclaw commented Sep 28, 2017

Next time, try using the auto-merge label.

@PetarKirov
Copy link
Member

@ibuclaw While in general I agree that using the auto-merge is a good practice, I don't see the problem with using "Rebase and merge" as long as it keeps a reference to the pull request, which it does by default. We were using this, in addition to "Squash and merge" successfully for dlang-tour before we deployed dlang-bot there too.

@wilzbach
Copy link
Member

before we deployed dlang-bot there too.

FWIW the bot supports the labels "auto-merge-squash" and "auto-merge-rebase" too, they aren't available here, because unfortunately there has been a precedent in which the squashing was "abused" :/

I don't see the problem with using "Rebase and merge" as long as it keeps a reference to the pull request, which it does by default. We were using this, in addition to "Squash and merge" successfully for dlang-tour

Yup. Many projects I worked on even had this as the only allowed merging strategy.

@mathias-lang-sociomantic
Copy link
Contributor

There is no merge commit.

Yep, that's the point of it actually. You should know that, because all D developpers around you are using this approach ;)

The point of not having a merge commit is to make the history more readable. A linear history is much easier to read than one polluted by merge commits. Compare ocean's history which use rebase approach:
2017-10-01-173040_1366x768_scrot
With dmd's:
2017-10-01-173346_1366x768_scrot

(Generated using log --color --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit)

Now if there's a technical issue or policy or strong preference from someone not to use rebase & merge I'll switch to merge approach of course, I was just surprised this could be an issue :)

@wilzbach
Copy link
Member

wilzbach commented Oct 1, 2017

Now if there's a technical issue or policy or strong preference from someone not to use rebase & merge I'll switch to merge approach of course

There such a policy (yet). In fact, e.g. we wanted to switch to squashing for a long time:

dlang/dlang-bot#64

(so AFAICT nothing is set into stones here, it's just missing time)

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.

5 participants