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

Exception message review #7201

Open
30 of 40 tasks
ajcvickers opened this issue Dec 7, 2016 · 6 comments
Open
30 of 40 tasks

Exception message review #7201

ajcvickers opened this issue Dec 7, 2016 · 6 comments

Comments

@ajcvickers
Copy link
Member

ajcvickers commented Dec 7, 2016

Exception messages have not always evolved as behavior has evolved. Some exception messages are now wrong, while others are now unhelpful. Proposal:

  • Review with a group of people making sure that messages are correct and deciding what to add or remove from the message. Avoid wordsmithing as a team since this is painful and slow.
  • Focus on messages commonly seen and consider adding forward links.
  • One person (e.g. @rowanmiller @roji) goes through and writes the messages
  • PR review for tweaks

Updated process:

  • Each @dotnet/efteam person will go through messages that they understand and check for content based on the checklist below.
    • This should not include wordsmithing or items in the style checklist--these will be done by one person per assembly
    • Each resource should be marked appropriately in the spreadsheet attached to the Teams channel
  • Once this has been completed, we will go through the remaining resources (based on the spreadsheet) as a team

Content checklist:

  • Does the message say what went wrong?
  • Does it have a second sentence indicating what to do about it?
  • Are we providing specific metadata names, ID values, etc. where helpful?
  • Does it expose sensitive data without sensitive data logging enabled?
  • Would a fwlink help?

Style checklist:

  • Use {entityType}.{property/navigation}' rather than '{property/navigation}' of entity type '{entityType}'. Note that this will change the order of parameters, and so call sites will need to be updated.
  • Specify attributes without single quotes and with the square brackets @smitpatel
  • Be consistent about OnModelCreating as the terminology for fluent API @AndriySvyryd
  • Use "navigation" rather than "navigation property" @AndriySvyryd
  • Check usage of {foreignKey} and for correct quoting and use foreignKeyProperties where appropriate @AndriySvyryd
  • Grammar, punctuation and casing @roji
  • Be consistent about methods being formatted as 'Method' @roji
  • Be consistent about types being formatted as 'Type'
  • Make sure all "non blah" are spelled "non-blah" @roji
  • Use "Entity Framework" when referring to EF @roji done @smitpatel
  • Consistently use "context instance" instead of "context" or "DbContext" @roji
  • Weak entity types should be referred to as entity types with a defining navigation @roji
  • Shared entity types should be referred to as shared-type entity types @roji
  • Is this still relevant? - "For ASP.NET WebForms bind to 'DbSet.ToList' or use Model Binding." @ajcvickers
  • Replace "please report it" with "file an issue" and a fwlink to new issue @smitpatel
  • Discuss whether null should be quoted and when (Exception message review #22519 (comment))

See also #22309, #22308, #22374

Specific messages to investigate:

@rowanmiller rowanmiller added this to the 2.0.0 milestone Jan 24, 2017
@rowanmiller rowanmiller self-assigned this Jan 24, 2017
@ajcvickers ajcvickers modified the milestones: Backlog, 2.0.0 Apr 17, 2017
@ajcvickers ajcvickers removed this from the Backlog milestone Sep 7, 2017
@ajcvickers ajcvickers reopened this Aug 17, 2020
@ajcvickers ajcvickers added this to the MQ milestone Aug 17, 2020
ajcvickers added a commit that referenced this issue Aug 28, 2020
Part of #7201

This PR contains the specific changes we made in the meeting, plus using ``{entityType}.{property/navigation}' in EFCore consistently.

I have also added other changes we discussed as check boxes in #7201 so that we can track this like we do API reviews.
ajcvickers added a commit that referenced this issue Aug 29, 2020
This PR contains the specific changes we made in the meeting, plus using ``{entityType}.{property/navigation}' in EFCore consistently.

The tt files has been updated to allow moving of parameters without changing the method signature and hence without needing to update all call sites, which is error prone.

I have also added other changes we discussed as check boxes in #7201 so that we can track this like we do API reviews.
@ajcvickers ajcvickers removed this from the MQ milestone Aug 29, 2020
@ajcvickers
Copy link
Member Author

@dotnet/efteam I think everything here is considered poachable. We can probably track this the same way as we do for API reviews.

(Removing from milestone so this is seen in triage on Monday.)

ajcvickers added a commit that referenced this issue Aug 30, 2020
This PR contains the specific changes we made in the meeting, plus using ``{entityType}.{property/navigation}' in EFCore consistently.

The tt files has been updated to allow moving of parameters without changing the method signature and hence without needing to update all call sites, which is error prone.

I have also added other changes we discussed as check boxes in #7201 so that we can track this like we do API reviews.
ajcvickers added a commit that referenced this issue Aug 31, 2020
This PR contains the specific changes we made in the meeting, plus using ``{entityType}.{property/navigation}' in EFCore consistently.

The tt files has been updated to allow moving of parameters without changing the method signature and hence without needing to update all call sites, which is error prone.

I have also added other changes we discussed as check boxes in #7201 so that we can track this like we do API reviews.
@smitpatel smitpatel added this to the 5.0.0 milestone Aug 31, 2020
@ajcvickers ajcvickers added this to the MQ milestone Sep 14, 2020
AndriySvyryd added a commit that referenced this issue Sep 27, 2020
AndriySvyryd added a commit that referenced this issue Sep 27, 2020
AndriySvyryd added a commit that referenced this issue Sep 28, 2020
ghost pushed a commit that referenced this issue Sep 28, 2020
roji added a commit that referenced this issue Oct 17, 2020
roji added a commit that referenced this issue Oct 19, 2020
ghost pushed a commit that referenced this issue Oct 20, 2020
AndriySvyryd added a commit that referenced this issue Oct 23, 2020
Improve exception messages

Part of #7201
AndriySvyryd added a commit that referenced this issue Oct 23, 2020
AndriySvyryd added a commit that referenced this issue Oct 23, 2020
AndriySvyryd added a commit that referenced this issue Oct 23, 2020
AndriySvyryd added a commit that referenced this issue Oct 23, 2020
AndriySvyryd added a commit that referenced this issue Oct 23, 2020
@AndriySvyryd AndriySvyryd removed their assignment Oct 23, 2020
ghost pushed a commit that referenced this issue Oct 23, 2020
@ajcvickers ajcvickers modified the milestones: MQ, 6.0.0 Dec 16, 2020
@AndriySvyryd AndriySvyryd modified the milestones: 6.0.0, MQ Jul 28, 2021
@smitpatel
Copy link
Member

InvalidSetSharedType is used when using string overload with incorrect type name, which is incorrect error message for the scenario

@smitpatel smitpatel removed their assignment Nov 10, 2022
@ajcvickers ajcvickers removed their assignment Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants