added new parameters to RepositoryRequest.cs #1096
Conversation
| CollaboratorAndOrganizationMember, | ||
|
|
||
| /// <summary> | ||
| /// Returns all repositories where user is owner,collaborator and organization member. |
There was a problem hiding this comment.
Should probably be or organization member
|
There seems to be a lot white space that could be cleaned up. |
|
@M-Zuber thanks for correction! i am fixing this. |
| /// Gets or sets the visibility property. | ||
| /// </summary> | ||
| /// <value> | ||
| /// The public. |
There was a problem hiding this comment.
Should be The visibility no?
|
@M-Zuber is it right now? if there are any mistakes further please let me know. thanks |
| @@ -103,4 +119,70 @@ public enum RepositorySort | |||
| [Parameter(Value = "full_name")] | |||
| FullName | |||
| } | |||
|
|
|||
|
|
|||
|
I apologize for being picky, it is an issue that I have. |
|
I can understand your point as this is an official repository of github api wrapper, these things(comment typos etc) really matter. I will push another commit when i will reach home.thanks again for correction. I appreciate it. |
|
😄 |
|
Hi @M-Zuber i need your help one more time 😄 please review this when you are free.thanks |
|
LGTM, but I do not have commit rights, so you will have to wait for one of the maintainers to confirm and merge. |
|
Regarding Unit Tests, I'd suggest it would be good to add a couple more similar to these:
eg CanFilterByVisibility |
|
@ryangribble unit tests 😕 it is difficult for me to write solid unit test i doubt that i will miss something in testing. it would be good if any one with testing knowledge should write test for these properties. what do you say? |
|
@M-Zuber @ryangribble thanks for the initial review 💎 @Sarmad93 I think writing these tests is actually easier that you suspect - we have a lot of tests already defined for The goal of these tests is to verify a small piece of behaviour - in this case, what we're interested in is the mapping of the parameters on the request to the URL that gets invoked by the client. The setup is what's called AAA syntax - Arrange, Act and Assert, and they're intended to be short, easy to read and quick to execute. Have a read of the links above, feel free to ask questions - but don't be afraid to take a shot at it 😁 |
|
yes @shiftkey you are absolutely right 👍 I am new to this open source work that's why i am little afraid for now. As i was reading the links above i realized that i have lot to learn. Your code base has nice documentation and nice language idioms especially linq queries are written very well. what we learn in our programming classes is totally different here in open source world. Thanks 😄 |
Welcome! We're all friendly here. We love having people help out in any way they see fit. We all have other responsibilities because of our work, so we can be slow to reply sometimes. But don't hesitate to ask questions. We want your first open source experience to be great so maybe you'll contribute again! 😄 |
|
Ok i will try to write unit test similar to above links tonight and will ask for the help if i get stuck. |
|
Good luck! |
|
Well done!
|
…sitory added new parameters to RepositoryRequest.cs
|
Noice!! |
|
Thank you everyone for helping me out especially @M-Zuber for initial review. You guys are really helpful. 😃 |
|
:blushing: |
|
@haacked is it really you in the gif? 😆 |
Yup! |
As per the issue #1065 opened by @shiftkey, i have added new parameters to RepositoryRequest class.