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

Fix Issue 16744: Add Typeof #5662

Merged
merged 1 commit into from
Jul 31, 2017
Merged

Fix Issue 16744: Add Typeof #5662

merged 1 commit into from
Jul 31, 2017

Conversation

MetaLang
Copy link
Member

This an alternative implementation to #4920

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @MetaLang! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close Bugzilla Description
16744 We should have a TypeOf template so that typeof can be used with templates like staticMap

@MetaLang MetaLang force-pushed the MetaLang-patch-1-3 branch 2 times, most recently from b26f48c to 53d4911 Compare July 29, 2017 02:04
This an alternative implementation to #4920
Copy link
Member

@PetarKirov PetarKirov left a comment

Choose a reason for hiding this comment

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

LGTM

@UplinkCoder
Copy link
Member

I approve this as well

@wilzbach wilzbach deleted the MetaLang-patch-1-3 branch July 31, 2017 18:49
@wilzbach wilzbach restored the MetaLang-patch-1-3 branch July 31, 2017 18:50
@wilzbach wilzbach deleted the MetaLang-patch-1-3 branch July 31, 2017 18:50
@wilzbach
Copy link
Member

@UplinkCoder - while this is a nice addition, it still is a new symbol and it this hasn't been approved by @andralex. On the contrary:

@jmdavis the simplest question is: since it's a trivial one-liner, why not just ask the user to do it if they have a need? We need to have a case for high frequency of use.

#4920 (comment)

-> #5664 reverts this temporarily, s.t. there's enough time to finish the discussion properly.

Sorry for the extra work @MetaLang

@MetaLang
Copy link
Member Author

I'm not quite sure how it works... should I re-submit the original PR?

@wilzbach
Copy link
Member

I'm not quite sure how it works... should I re-submit the original PR?

I think so - you might need to be a bit careful on the rebase after the revert has been merged.

@andralex
Copy link
Member

Yah, thanks @wilzbach I do want us to think this over a little better

MetaLang added a commit that referenced this pull request Oct 18, 2017
Previous PR: #5662. I'm resubmitting this after it was reverted in #5664.

This an alternative implementation to #4920.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants