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

Revert "Deprecate alias this for classes v2" #15326

Merged
merged 1 commit into from Jul 17, 2023

Conversation

WalterBright
Copy link
Member

Reverts #14812

Well, that was a disaster. My fault.

@dlang-bot
Copy link
Contributor

dlang-bot commented Jun 16, 2023

Thanks for your pull request, @WalterBright!

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.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "stable + dmd#15326"

@WalterBright
Copy link
Member Author

I need to push a change to this PR. How do I do it? googling has failed me!

@WalterBright
Copy link
Member Author

Here is my attempt to fix it:

git pull upstream revert-14812-deprecate_alias_this_for_classes
git checkout revert-14812-deprecate_alias_this_for_classes
cd compiler/src/dmd
edit dtoh.d
git status
git commit --amend -a --no-edit
git push -f origin revert-14812-deprecate_alias_this_for_classes

These commands succeeded, but where did my changes go? They're not here.

@RazvanN7
Copy link
Contributor

What I typically do (assuming that you are now on master/main branch):

git fetch your_fork_name revert-14812
git checkout  revert-14812
... edit files ...
git add $(edited_files)
git commit -m "Revert alias this deprecation for classes"
git push -f your_fork_name revert-14812

@WalterBright
Copy link
Member Author

What is my fork name?

@WalterBright
Copy link
Member Author

This is the usual way I amend one of my pull requests:

git checkout revert-14812-deprecate_alias_this_for_classes
cd compiler/src/dmd
edit dtoh.d
git status
git commit --amend -a --no-edit
git push -f origin revert-14812-deprecate_alias_this_for_classes

Why did this not work in this case? Where did the git push send it? To purgatory?

@dkorpel
Copy link
Contributor

dkorpel commented Jun 16, 2023

These commands succeeded, but where did my changes go?

When you use the 'revert' button on GitHub, it creates a branch in the dlang/dmd repository. You pulled from this with git pull upstream, but you pushed to origin with git push -f origin, so it got sent to your WalterBright/dmd fork: https://github.com/WalterBright/dmd/tree/revert-14812-deprecate_alias_this_for_classes

@dkorpel
Copy link
Contributor

dkorpel commented Jun 16, 2023

Perhaps open a new pull request from your fork

@ErnyTech
Copy link
Contributor

The deprecation of alias this for the class was a disaster mainly because there was no viable alternative.

I'm happy with this revert but I think it would be good later to think about how to solve the problems that were with alias this or provide an alternative feature

@WalterBright
Copy link
Member Author

@dkorpel so I need to push it to upstream instead of origin?

@WalterBright WalterBright force-pushed the revert-14812-deprecate_alias_this_for_classes branch from a87d172 to b4ebb8f Compare June 17, 2023 03:43
@WalterBright
Copy link
Member Author

Looks like pushing it to upstream worked

@WalterBright
Copy link
Member Author

The test suite errors look irrelevant.

@dkorpel dkorpel force-pushed the revert-14812-deprecate_alias_this_for_classes branch from b4ebb8f to c184843 Compare July 1, 2023 12:57
@dkorpel dkorpel changed the base branch from master to stable July 1, 2023 12:59
@dkorpel dkorpel force-pushed the revert-14812-deprecate_alias_this_for_classes branch 2 times, most recently from ef276c0 to ddd1605 Compare July 1, 2023 13:09
@dkorpel dkorpel force-pushed the revert-14812-deprecate_alias_this_for_classes branch from ddd1605 to eee96d9 Compare July 5, 2023 11:09
@dkorpel dkorpel force-pushed the revert-14812-deprecate_alias_this_for_classes branch from eee96d9 to 24af6fe Compare July 14, 2023 15:49
@dkorpel dkorpel force-pushed the revert-14812-deprecate_alias_this_for_classes branch from 24af6fe to 5de1db8 Compare July 17, 2023 10:52
Copy link
Contributor

@dkorpel dkorpel left a comment

Choose a reason for hiding this comment

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

Now that stable is merged, no more spurious failures

@dkorpel dkorpel merged commit 5387bc4 into stable Jul 17, 2023
43 of 44 checks passed
@dkorpel dkorpel deleted the revert-14812-deprecate_alias_this_for_classes branch July 17, 2023 15:52
dkorpel added a commit that referenced this pull request Jul 18, 2023
* druntime: Restrict some `pragma(inline, false)` kludges to DMD only

They appear related to DMD's inlining at the AST level; LDC at least
doesn't need them, so let the optimizer decide for non-DMD backends.

* Fix little C++ header regression

* Expose VarArg.KRvariadic to C++ headers

* Revert "Deprecate alias this for classes v2 (#14812)" (#15326)

This reverts commit af7817b.

* Add changelog for catch qualifier deprecation & update release no

---------

Co-authored-by: Martin Kinkelin <noone@nowhere.com>
Co-authored-by: Walter Bright <WalterBright@users.noreply.github.com>
Co-authored-by: Nick Treleaven <ntrel002@gmail.com>
@ibuclaw ibuclaw mentioned this pull request Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants