-
Notifications
You must be signed in to change notification settings - Fork 228
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
Removing github action warnings. #505
Removing github action warnings. #505
Conversation
BigUstad
commented
Dec 15, 2020
- Fixing missing closing/opening XML tags.
- Removing instances of Parameter (IRestResponse, RestRequest), if possible
- In Functional Tests removing async for some calls.
- Marking some functions as obsolete if missing, changing accordingly.
- Removing unused, initialized variable.
- Removing unnecessary 'using' statements.
866f064
to
e55f9d9
Compare
698723f
to
9d44901
Compare
9d44901
to
6bc7e7f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still GitHub Actions
warnings in Minio/ApiEndpoints/BucketOperations.cs
file:
Example:
Check warning on line 90 in Minio/ApiEndpoints/BucketOperations.cs
GitHub Actions
/ build (Release)
Minio/ApiEndpoints/BucketOperations.cs#L90
The method 'MakeBucketAsync' do not need to use async/await.
See Minio/ApiEndpoints/BucketOperations.cs
file under Files Changed
of this PR.
I also see total 67 warnings during dotnet build
. This list also includes the above GitHub Actions
warnings.
They look like all belong to your to be fixed list in this PR's description/
Here is the short list of those warnings.
(32) The method '<method-name>' do not need to use async/await
(28) MinioClient.<object-builder-name(...)> is obsolete
(4) CopyToAsync should be used instead of stream.CopyTo
(2) Initialize is a fire & forget async void method
(1) This async method lacks 'await' operators and will run synchronously.
The "do not need to use async/await" is definitely wrong as |
d58870c
to
7df482a
Compare
@ebozduman Please try with the latest patchset. |
d3bb878
to
f223445
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Priyank said he'll address the last 2 warnings about stream.CopyTo
.
So, LGTM
29291f1
to
5162fff
Compare
5162fff
to
589a9b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM