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

Fixed some CS1572, 1573, 1574 and 1591 warnings #2063

Merged
merged 16 commits into from
Apr 21, 2020
Merged

Fixed some CS1572, 1573, 1574 and 1591 warnings #2063

merged 16 commits into from
Apr 21, 2020

Conversation

d3fkn1ght
Copy link
Contributor

@d3fkn1ght d3fkn1ght commented Apr 17, 2020

Description of Change

Fixed some CS1572, 1573, 1574 and 1591 warnings

Bugs Fixed

API Changes

None

Behavioral Changes

None

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • [] Changes adhere to coding standard

Copy link
Contributor Author

@d3fkn1ght d3fkn1ght left a comment

Choose a reason for hiding this comment

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

Thanks for adding the newlines. Definitely improves readability.

src/Prism.Core/Common/ParametersBase.cs Outdated Show resolved Hide resolved
src/Prism.Core/Common/ParametersBase.cs Outdated Show resolved Hide resolved
src/Prism.Core/Common/ParametersBase.cs Outdated Show resolved Hide resolved
src/Prism.Core/Common/ParametersBase.cs Outdated Show resolved Hide resolved
src/Prism.Core/Common/ParametersBase.cs Outdated Show resolved Hide resolved
src/Prism.Core/Modularity/IModuleInfo.cs Outdated Show resolved Hide resolved
src/Prism.Core/Modularity/IModuleInfo.cs Outdated Show resolved Hide resolved
src/Prism.Core/Modularity/IModuleInfo.cs Outdated Show resolved Hide resolved
src/Prism.Core/Modularity/IModuleInfo.cs Outdated Show resolved Hide resolved
src/Prism.Core/Modularity/IModuleInfoGroup.cs Outdated Show resolved Hide resolved
Adding comments per request of Dan Siegel
I have changed the file in an attempt to reduce the confusion identified by Dan Siegel
I have added the following comment to the key parameters
Added comment to the <param> tag for method documentation::
<param name="key">The key for the value to be returned</param>

Added following comment tot he <returns> tag for method documentation:
Returns a matching parameter of <typeparam name="T" /> if one exists in the Collection
Modified <param> tags on a few methods that check existence of keys/values or return values
Fixed Indentation
Updated comment for InitializationMode
Added <ref> clarification and <remarks> tag as requested by Dan Siegel
Update comment on ModuleState
Add ref summary info as reqeusted
@d3fkn1ght
Copy link
Contributor Author

I have made all the requested changes. Please review at your convenience.

Copy link
Contributor Author

@d3fkn1ght d3fkn1ght left a comment

Choose a reason for hiding this comment

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

All requested changes have been implemented

Copy link
Contributor Author

@d3fkn1ght d3fkn1ght left a comment

Choose a reason for hiding this comment

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

I have incorporated the requested changes, please let me know if there is anything I missed or any other required changes

Copy link
Member

@dansiegel dansiegel left a comment

Choose a reason for hiding this comment

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

Looking good just need some updates for the ParametersBase...

src/Prism.Core/Common/ParametersBase.cs Outdated Show resolved Hide resolved
src/Prism.Core/Common/ParametersBase.cs Outdated Show resolved Hide resolved
src/Prism.Core/Common/ParametersBase.cs Outdated Show resolved Hide resolved
src/Prism.Core/Common/ParametersBase.cs Outdated Show resolved Hide resolved
src/Prism.Core/Common/ParametersBase.cs Outdated Show resolved Hide resolved
src/Prism.Core/Common/ParametersBase.cs Outdated Show resolved Hide resolved
src/Prism.Core/Common/ParametersBase.cs Outdated Show resolved Hide resolved
I have added text to some of the tags as requested by Dan Siegel
@d3fkn1ght
Copy link
Contributor Author

I believe I have made all the required changes

I have added text to all the empty tags I found
@d3fkn1ght
Copy link
Contributor Author

Hopefull all the tags have text now. I apologize for missing that.

public IEnumerable<string> Keys =>
_entries.Select(x => x.Key);

/// <summary>
/// Adds the <paramref name="key" /> and <paramref name="value" /> to the KeyValuePair<string,object> collection
Copy link
Member

Choose a reason for hiding this comment

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

let's remove the paramref's from here...

Removed paramref
Copy link
Contributor Author

@d3fkn1ght d3fkn1ght left a comment

Choose a reason for hiding this comment

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

Removed paramref

Copy link
Member

@dansiegel dansiegel left a comment

Choose a reason for hiding this comment

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

Looks good thanks for the PR!

@dansiegel dansiegel merged commit af4811b into PrismLibrary:master Apr 21, 2020
@d3fkn1ght d3fkn1ght deleted the documentation branch April 21, 2020 15:03
@lock
Copy link

lock bot commented May 5, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants