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

Less logging of DstvExceptions to the console #5

Merged
merged 2 commits into from
Sep 1, 2023

Conversation

EjaYF
Copy link
Contributor

@EjaYF EjaYF commented Jul 13, 2023

This fix addresses issues where DstvExceptions appear in the console log unnecessary.

@EjaYF EjaYF requested review from tluijken and Mstrdk July 13, 2023 11:10
@@ -17,6 +17,7 @@ public static async Task<IEnumerable<DstvElement>> GetElementsAsync(ReaderContex
foreach (var holeNote in holeBlocks.SelectMany(holeList => holeList))
try
{
if (holeNote.Equals(ContourType.BO.ToString(), StringComparison.Ordinal)) continue;
Copy link

Choose a reason for hiding this comment

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

I'd prefer the use of the nameof operator in these cases (e.g. nameof(ContourType.BO)), but that's pretty personal...

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Me too!

@@ -17,6 +17,7 @@ public static async Task<IEnumerable<DstvElement>> GetElementsAsync(ReaderContex
foreach (var holeNote in holeBlocks.SelectMany(holeList => holeList))
try
{
if (holeNote.Equals(ContourType.BO.ToString(), StringComparison.Ordinal)) continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree

@@ -55,6 +57,7 @@ public static async Task<IEnumerable<DstvElement>> GetElementsAsync(ReaderContex
foreach (var bendNote in bendBlocks.SelectMany(bendList => bendList))
try
{
if (bendNote.Equals(ContourType.KA.ToString(), StringComparison.Ordinal)) continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like there is a lot of repetition going on here...should we make a generic function for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've simplified the code

@EjaYF EjaYF requested a review from tluijken August 25, 2023 14:03
@martijn00
Copy link
Member

@EjaYF Can you rebase and resolve the conflicts?

@EjaYF EjaYF force-pushed the feature/ErrorConsoleLogFix branch from 9aa1bbd to 0300106 Compare August 30, 2023 14:32
@tluijken tluijken merged commit 41acb38 into develop Sep 1, 2023
2 checks passed
@tluijken tluijken deleted the feature/ErrorConsoleLogFix branch September 1, 2023 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants