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
Rename EquivalencyAssertionOptions
to EquivalencyOptions
#2414
Rename EquivalencyAssertionOptions
to EquivalencyOptions
#2414
Conversation
EquivalencyAssertionOptions
to EquivalencyOptions
Qodana for .NET5 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/qodana-action@v2023.2.8
with:
upload-result: true Contact Qodana teamContact us at qodana-support@jetbrains.com
|
aee9846
to
005f78a
Compare
I'm all for this change, but I suspect @jnyrup may have a different perspective on breaking changes like these. |
You read me like an open book.
To extend on this, I'm quite conservative when it comes to breaking changes, even between major versions. I do agree that the new name is better, but I don't think the change pull it's own weight. If I should try being a tiny bit pragmatic for once: |
As a user of this library, I am often glad about this approach, as it simplifies updates. Should I leave the pull request open for now and wait for #2253? As an alternative, if you prefer to not rename the classes at all, I could also just remove the code comment. |
It will. As I have it envisioned in my brain, it will be big time breaking change ;-) |
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.
#2413 will also cause a related breaking change by adding members to the public interface IEquivalencyAssertionOptions
.
Let's do this 🚀
…renceEquivalencyAssertionOptions to SelfReferenceEquivalencyOptions
005f78a
to
ee7b7e5
Compare
Pull Request Test Coverage Report for Build 6755429589
💛 - Coveralls |
What is wrong with the qodana scan? 🤔 |
I am not sure. I had similar problems with in #2431... |
IMHO it has nothing to do with the code itself? |
I think we need to look at that Qodana rule |
While working on #2413 I stumbled over this code comment to rename
EquivalencyAssertionOptions
toEquivalencyOptions
. As we are currently working on the next major version, I thought it a good time to implement it.IMPORTANT
./build.sh --target spellcheck
or.\build.ps1 --target spellcheck
before pushing and check the good outcome