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

Offer no suggestion for single constructor #5747

Merged
merged 2 commits into from
Feb 28, 2023

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Feb 23, 2023

Pull Request Description

Resolving #5055 - avoid putting single constructor into suggestion database.

Important Notes

Another way to fix #5055 is to keep the single constructor information in the suggestion database and let the IDE filter that out.

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the
    Scala,
    style guides.
  • All code has been tested:
    • Unit tests have been written where possible.

@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Feb 23, 2023
@JaroslavTulach JaroslavTulach requested a review from 4e6 as a code owner February 23, 2023 09:19
@JaroslavTulach JaroslavTulach self-assigned this Feb 23, 2023
@JaroslavTulach
Copy link
Member Author

@jdunkerley seems to be OK with this change. What do @4e6 and @hubertp think?

Copy link
Collaborator

@hubertp hubertp left a comment

Choose a reason for hiding this comment

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

So what about Pair.Value? that takes two parameters, should that not be reported in suggestion DB either?

@JaroslavTulach
Copy link
Member Author

So what about Pair.Value? that takes two parameters, should that not be reported in suggestion DB either?

Pair.Value is a great example and there may be others.

Pair.Value is marked as UNSTABLE - users are likely supposed to use Pair.new. E.g. not including the constructor in suggestion DB in this case is probably OK... if that's what @Frizi and @jdunkerley want.

Copy link
Collaborator

@hubertp hubertp left a comment

Choose a reason for hiding this comment

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

I would probably limit this to a hardcoded Value to avoid some unexpected behaviour. Up to you if you want to include that.

@jdunkerley
Copy link
Member

So what about Pair.Value? that takes two parameters, should that not be reported in suggestion DB either?

Pair.Value is a great example and there may be others.

Pair.Value is marked as UNSTABLE - users are likely supposed to use Pair.new. E.g. not including the constructor in suggestion DB in this case is probably OK... if that's what @Frizi and @jdunkerley want.

Pair.Value should indeed be hidden from users. Pair.new is supposed to be the method that users create it.

In general, the goal was that we wouldn't show a drop down for things with a single constructor as that (probably) isn't what we want.

@JaroslavTulach JaroslavTulach added the CI: Ready to merge This PR is eligible for automatic merge label Feb 28, 2023
@mergify mergify bot merged commit 0536eeb into develop Feb 28, 2023
@mergify mergify bot deleted the wip/jtulach/NoTagValuesForSingles_5055 branch February 28, 2023 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dropdown for Table.join right is wrong
3 participants