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

[Chore]: Add XML comments for visible types or members #733

Open
Dor-bl opened this issue Jan 26, 2024 · 7 comments
Open

[Chore]: Add XML comments for visible types or members #733

Dor-bl opened this issue Jan 26, 2024 · 7 comments
Labels
Documentation good first issue First time contributers could start with that HelpWanted

Comments

@Dor-bl
Copy link
Collaborator

Dor-bl commented Jan 26, 2024

Many Complier Warnings CS1591 are found in the project.
Any assistance with those would be great!
https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/cs1591?f1url=%3FappId%3Droslyn%26k%3Dk(CS1591)
image

some examples:

src\Appium.Net\Appium\Interfaces\IHidesKeyboard.cs(42,33): Warning CS1584: XML comment has syntactically incorrect cref attribute 'true'VSBuild |   |  
-- | -- | --
  | src\Appium.Net\Appium\Interfaces\IHidesKeyboard.cs(42,33): Warning CS1658: Identifier expected; 'true' is a keyword. See also error CS1041.VSBuild |   |  
  | src\Appium.Net\Appium\Interfaces\IHidesKeyboard.cs(42,74): Warning CS1584: XML comment has syntactically incorrect cref attribute 'false'VSBuild |   |  
  | src\Appium.Net\Appium\Interfaces\IHidesKeyboard.cs(42,74): Warning CS1658: Identifier expected; 'false' is a keyword. See also error CS1041.VSBuild |   |  
  | src\Appium.Net\Appium\MobileBy.cs(69,24): Warning CS1584: XML comment has syntactically incorrect cref attribute 'https://developer.android.com/intl/ru/training/accessibility/accessible-app.html'VSBuild |   |  
  | src\Appium.Net\Appium\MobileBy.cs(71,24): Warning CS1584: XML comment has syntactically incorrect cref attribute 'https://developer.apple.com/library/ios/documentation/UIKit/Reference/UIAccessibilityIdentification_Protocol/index.html'VSBuild |   |  
  | src\Appium.Net\Appium\MobileBy.cs(80,24): Warning CS1584: XML comment has syntactically incorrect cref attribute 'http://developer.android.com/intl/ru/tools/testing-support-library/index.html#uia-apis'VSBuild |   |  
  | src\Appium.Net\Appium\AppiumCommand.cs(277,1): Warning CS1570: XML comment has badly formed XML -- 'Expected an end tag for element 'summary'.'VSBuild |   |  
  | src\Appium.Net\Appium\MobileBy.cs(80,101): Warning CS1658: Unexpected character '#'. See also error CS1056.VSBuild |   |  
  | src\Appium.Net\Appium\MobileBy.cs(89,24): Warning CS1584: XML comment has syntactically incorrect cref attribute 'http://developer.android.com/intl/ru/tools/testing-support-library/index.html#uia-apis'VSBuild


@Dor-bl Dor-bl added HelpWanted good first issue First time contributers could start with that labels Jan 26, 2024
@dragojs
Copy link

dragojs commented Feb 9, 2024

I would like to possibly help with this; I spotted some that are obvious to amend but the majority are about Missing XML comment for member.
Ideally, do you envision everything having an adequate comment or do you plan to disable the warning in certain classes / for certain scenarios/members etc. ?

@Dor-bl
Copy link
Collaborator Author

Dor-bl commented Feb 10, 2024

Hi @dragojs.
Thank you for reaching out first of all, any help would do.
The idea is to add comments whenever possible, rather than just disable the warnings.
You can start with a small PR at first and I assume that with time you will be able to send bigger PR's.

dragojs pushed a commit to dragojs/dotnet-client that referenced this issue Feb 10, 2024
dragojs pushed a commit to dragojs/dotnet-client that referenced this issue Feb 10, 2024
dragojs added a commit to dragojs/dotnet-client that referenced this issue Feb 13, 2024
@sambrandt
Copy link

If you're still looking for some help, I'd love to contribute. I'm going to fork the repo and take a look tonight. Let me know how I can best work with you.

@dragojs
Copy link

dragojs commented Feb 14, 2024

There are some plain obvious fixes which I touched right away and knocked out about 100 warnings but really I just scratched the surface.
For most of the others you really have to scratch your head to figure out what a suitable comment is. I'd probably look at how other clients tackled this; can't really think of a better approach

@Dor-bl
Copy link
Collaborator Author

Dor-bl commented Mar 3, 2024

@sambrandt @dragojs I'm not sure where you guys stand with this, but I came across a relatively easy fix for warnings related to cref.
you can check out this PR for examples:
#752

dragojs added a commit to dragojs/dotnet-client that referenced this issue Mar 3, 2024
@dragojs
Copy link

dragojs commented Mar 3, 2024

Went the same route; haven't had as much time as I wished to dedicate to this; there are several few improvements on a branch of mine that refers to this, I can possibly try to PR later this week.
Looked a bit in the java client and there's either barely any comments or I'm looking in the wrong place. The python client however does have some comments to serve as inspiration

@sambrandt
Copy link

Hey! Sorry, I started to work, and then life exploded a bit. I'm hoping that this week I can get some traction on this. Thanks for the reference, I'll take a look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation good first issue First time contributers could start with that HelpWanted
Projects
None yet
Development

No branches or pull requests

3 participants