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

Added support markdown table headers in release tracking markdown files #6566

Conversation

peteraritchie
Copy link
Contributor

Addresses issue #5866

@peteraritchie peteraritchie requested a review from a team as a code owner April 2, 2023 22:15
@codecov
Copy link

codecov bot commented Apr 2, 2023

Codecov Report

Merging #6566 (0f4e011) into main (916b9a6) will decrease coverage by 0.02%.
The diff coverage is 87.50%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6566      +/-   ##
==========================================
- Coverage   96.43%   96.41%   -0.02%     
==========================================
  Files        1372     1374       +2     
  Lines      320265   320382     +117     
  Branches    10293    10314      +21     
==========================================
+ Hits       308844   308909      +65     
- Misses       8967     9005      +38     
- Partials     2454     2468      +14     

@@ -545,8 +545,12 @@ class MyAnalyzer : DiagnosticAnalyzer
[InlineData("", ReleaseTrackingHelper.TableHeaderNewOrRemovedRulesLine1 + BlankLine + "Id1 | Category1 | Warning |")]
// Missing TableHeaderLine1 in unshipped
[InlineData("", ReleaseTrackingHelper.TableTitleNewRules + BlankLine + ReleaseTrackingHelper.TableHeaderNewOrRemovedRulesLine2 + BlankLine + "Id1 | Category1 | Warning |", 2)]
// Missing TableHeaderLine1 in unshipped 2
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of just having an opaque suffix 2, can you add a more detailed comment that this is testing the regex matching with xyz aspects of the entry? Same for InlineData additions below.

Copy link
Contributor

@mavasani mavasani left a comment

Choose a reason for hiding this comment

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

LGTM, with minor suggestion on improving the comment in added tests. I can merge once you enhance the comments. Thanks!

@mavasani mavasani enabled auto-merge April 25, 2023 13:58
@mavasani mavasani merged commit 429d834 into dotnet:main Apr 25, 2023
@github-actions github-actions bot added this to the vNext milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants