search

ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.38k stars 1.64k forks source link

Applied the newer code analysis rules. #1729

Closed EngRajabi closed 1 year ago

EngRajabi commented 1 year ago

add .net code analyzer and improve code. https://learn.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#code-analysis-properties https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/overview?tabs=net-7

raman-m commented 1 year ago

Mohsen, Thanks for submitted PR! But...

P.S. ⚠️ Focus on current open PRs please!

raman-m commented 1 year ago

Another problem:

There is no build at all!

You're requesting merge from a feature branch which was created from a branch which has commits which don't belong to target branch! That's why GitHub cannot decide which workflow should be run for the current feature branch to trigger building pipeline.

By other words, you have develop branch in your forked repo which is not identical as ThreeMammals:develop! The comparison diff of both branches must be zero! You did the mistake while merging PR 9 in your repo. You pressed Merge pull request button instead of Rebase and merge one. As I pointed your attention in this comment, the best approach is just watching the changes in head repo and press Sync fork button which will perfectly synchronize your develop branch to head repo one! So, both develop branches will be identical always if you use Sync fork button! Is it too difficult to understand? I create PRs for you to sync develop just to point your attention that your develop branch is outdated.

How to recover from blocked state

  1. Run this script to recover your develop branch:

    git switch develop
git reset --hard HEAD~1
git push --force

  2. After that press Sync fork for develop branch. The comparison diff must be zero after the sync. If Yes, both branches are identical.

  3. After that rebase your feature branch onto develop branch (better in Visual Studio) and make force push to origin:

    git checkout feat_addanalyzor # it must be checked out during rebasing in Visual Studio
git rebase --onto develop feat_addanalyzor # but it is better to do rebasing in Visual Studio
git push --force
raman-m commented 1 year ago

Dear Mohsen, Don't do such mistakes next time plz! I'm a little tired of your mistakes in all your PRs. It is pretty clear thing to try to follow development process.

raman-m commented 1 year ago

You must create an issue first, or open discission thread in Discussions space!

So, this PR is marked as draft.

EngRajabi commented 1 year ago

Mohsen, Thanks for submitted PR! But...

  • I assigned low priority for such kind of styling improvements
  • Next time a PR without attached issue or linked discussion will be closed automatically without an explanation, for now I will keep it open but with low priority
  • You've expressed your personal vision of stylings & conventions, so I would say it must be discussed first
  • You have to focus on current open PRs by you!!! Why do you create the next one?
  • If the author has 1-2 open PRs the next PRs will be closed automatically because in my opinion contributor cannot keep another PRs opened ignoring actual work on them

P.S. ⚠️ Focus on current open PRs please!

These changes include the following improvements that affect performance، Security improvements, productivity and...

EngRajabi commented 1 year ago

Another problem:

There is no build at all!

You're requesting merge from a feature branch which was created from a branch which has commits which don't belong to target branch! That's why GitHub cannot decide which workflow should be run for the current feature branch to trigger building pipeline.

By other words, you have develop branch in your forked repo which is not identical as ThreeMammals:develop! The comparison diff of both branches must be zero! You did the mistake while merging PR 9 in your repo. You pressed Merge pull request button instead of Rebase and merge one. As I pointed your attention in this comment, the best approach is just watching the changes in head repo and press Sync fork button which will perfectly synchronize your develop branch to head repo one! So, both develop branches will be identical always if you use Sync fork button! Is it too difficult to understand? I create PRs for you to sync develop just to pint your attention that your develop branch is outdated.

How to recover from blocked state

  1. Run this script to recover your develop branch:
git switch develop
git reset --hard HEAD~1
  1. After that press Sync fork for develop branch. The comparison diff must be zero after the sync. If Yes, both branches are identical.
  2. After that rebase your feature branch onto develop branch (better in Visual Studio) and make force push to origin:
git checkout feat_addanalyzor # it must be checked out during rebasing in Visual Studio
git rebase --onto develop feat_addanalyzor # but it is better to do rebasing in Visual Studio
git push --force

I merged your Develop branch on my Develop branch.

raman-m commented 1 year ago

@EngRajabi commented on Oct 13 I merged your Develop branch on my Develop branch.

OK, you've merged. But did you follow the instructions? Did you resolve the problem of feature branch building? Did you understand the problem?

You did the same mistake twice! You don't follow development process in this PR!

raman-m commented 1 year ago

Look at Files changed, and it has nothing:

These merge commits were added into this branch cleanly.

Your feature branch is invalid, it's broken now! This PR will be closed because of zero changes.

raman-m commented 1 year ago

@EngRajabi commented on Oct 13 These changes include the following improvements that affect performance، Security improvements, productivity and...

I am tired! Sorry! And I don't care about your requests anymore...

raman-m commented 1 year ago

EngRajabi commented on Oct 12

Hi @ggnaegi ! I know you love code quality tools, static analyzers and code scanners... How useful are these links to read and understand a benefit for our project?

I use Visual Studio of latest version, and IDE just has built-in static analyzers like CA and IDE messages during local builds. I try to fix all reported issues. Also, we have enabled StyleCop analyzer with predefined rules, which is pretty useful. So, trying to fix all reported errors & warnings & messages makes the code much better. I am just cannot get, what benefits will we earn if we review the project by approaches written in the docs of these links? What is the benefit?