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

Encode characters to prevent invalid XML comments #837

Merged
merged 3 commits into from
Feb 12, 2023
Merged

Encode characters to prevent invalid XML comments #837

merged 3 commits into from
Feb 12, 2023

Conversation

kmgallahan
Copy link
Contributor

@kmgallahan kmgallahan commented Feb 11, 2023

Proposed change

See #836

SecurityElement.Escape was just added in 2 places.

Note: I did not attempt a build or to run any tests. I think the parentheses are in the right places...

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the [development checklist][dev-checklist]
  • The code compiles without warnings (code quality chek)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@codecov
Copy link

codecov bot commented Feb 11, 2023

Codecov Report

Base: 80.98% // Head: 80.98% // No change to project coverage 👍

Coverage data is based on head (000c8e0) compared to base (09decce).
Patch coverage: 50.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #837   +/-   ##
=======================================
  Coverage   80.98%   80.98%           
=======================================
  Files         159      159           
  Lines        4107     4107           
  Branches      406      406           
=======================================
  Hits         3326     3326           
  Misses        620      620           
  Partials      161      161           
Flag Coverage Δ
unittests 80.98% <50.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...odeGenerator/CodeGeneration/SyntaxFactoryHelper.cs 97.10% <50.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@FrankBakkerNl FrankBakkerNl left a comment

Choose a reason for hiding this comment

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

I think there are more places where we add xml comments that could use this. Did you check for that?

@kmgallahan
Copy link
Contributor Author

kmgallahan commented Feb 12, 2023

I only did a couple of quick searches for things like /// and summary. From that and poking the other files in this location it didn't immediately appear that there were other places, but I'm not familiar with the code base.

@helto4real helto4real merged commit c263a7f into net-daemon:dev Feb 12, 2023
@kmgallahan kmgallahan deleted the patch-2 branch February 13, 2023 02:27
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.

Invalid XML comments can be created due to not encoding special characters
3 participants