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.

Sign up for GitHub

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

Jump to bottom

Warnings again #796

Merged
merged 28 commits into from
Jul 7, 2023
Merged

Warnings again #796

merged 28 commits into from
Jul 7, 2023

Conversation

martijn00
Copy link
Contributor

@ebozduman lots of file movements here to better structure the code and remove warnings. Can you help and add to this PR with fixes for the warnings? After this gets merged the analyzer can be enabled by default!

@ebozduman
Copy link
Collaborator

@martijn00 ,

The lint changes are ready to be pushed but before doing that I noticed there are total 363 files, all marked as rewrite by the git commit command. However this PR shows only 278 files as being modified. I think we need to understand this before pushing the changes.

..... after a while ......

OK. I think this is because you work in a Windows environment and even the unmodified files come with a change since every lilne ends with CRLF, which are automatically replaced with LF by git. Although it is kind of late as they became part of the PR. This should have been taken care of before the PR is created.

So, please make sure the end of line character in your VS Code settings, Files:Eol, is set to \n.

@martijn00
Copy link
Contributor Author

@ebozduman This branch now builds and has the lowest warning setting enabled. It is ready to be merged and in new PR's I can set the warnings stricter.

@martijn00
Copy link
Contributor Author

@ebozduman Can you run CI again?

ebozduman
ebozduman approved these changes Jul 7, 2023
View reviewed changes
Copy link
Collaborator

@ebozduman ebozduman left a comment

Choose a reason for hiding this comment

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

LGTM
(Too many files make the review process very difficult and increases the chance of some unwanted changes to slip through inadvertently.)

@ebozduman ebozduman merged commit 56d687f into minio:master Jul 7, 2023
ebozduman pushed a commit to ebozduman/minio-dotnet that referenced this pull request Jul 7, 2023
@martijn00
Warnings again (minio#796)
2e0a8c3 
* Fix more warnings

* Nuget update

* Extend interface

* Move progres

* Namespace changes

* Format

* Renames

* Moves

* More moves

* More warnings

* Use new package for reactive

* Add editor config

* Allow 0

* Fix warnings

* Warnings as error

* Lint changes

* Revert "Drawingback Lint changes"

This reverts commit ed1ba1b.

* Fix lines

* Add editor config update

* More editor

* More editorconfig

* Format

* Format

* Format

* Fixes

* Format

* Format

* Runs bucket notification test by itself

---------

Co-authored-by: Ersan Bozduman <ersanbozduman@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebozduman ebozduman ebozduman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@martijn00 @ebozduman