Revert #9393 on RC2.1.0 branch#9420
Merged
QilongTang merged 3 commits intoDynamoDS:RC2.1.0_masterfrom Jan 14, 2019
Merged
Conversation
* add /new to GitHubBugReportingLink url * add bug report title to .resx * add support for pre-filled new Github issue text when reporting * Revert resx un-necessary changes * simplify query construction since we make a new Uri, it would never have any existing query parameters * rename variables for legibility * add fallback values for text to prevent null exceptions * invert IF logic to bail early * add XML comment for public method * get rid of conditional logic ✌️ * scaffold unit tests for CrashReporting * add CrashReportingTests.cs to DynamoCoreWpfTests proj * remove public access modifiers * add logged out fallback and make tab detection case-insensitive * move issue text & url formatting to new CrashUtilities.cs file * add test for long stack trace * Revert "Revert resx un-necessary changes" This reverts commit 01fc93c. * add description to issue crash report * fix XML comments * further formatting to crash report * Include issue details from old template * additional new line required * remove browser interaction from unit tests * decrement Dynamo version to make release * comment spelling * spelling - GithhubCrashReportBody becomes GitHubCrashReportBody
This reverts commit dfb7ddf.
Member
|
LGTM (pr is busted because of eng ops job, please restart with correct params) |
Contributor
|
@mjkkirschner Resource changes in it? I will let localization team know |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This PR reverts #9393 as QA testing revealed the following regression. When attempting to submit a crash report from Dynamo, while not logged in to GitHub, it is easily possible to exceeding the maximum URI length leading to
414 Request-URI Too Largeerror in the browser. Previously users were taken tohttps://github.com/DynamoDS/Dynamo/issuesno matter what which does not require auth.The team is concerned this will lead to users failing to report errors if they are not always logged in to GitHub. We will revert this PR in RC2.1 but leave it in Master and address the issue in DYN-1194.
Issue/Solution
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@QilongTang @mjkkirschner @smangarole
FYIs
@Racel @radumg