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

Update verify #151

Merged
merged 6 commits into from
Jun 10, 2024
Merged

Update verify #151

merged 6 commits into from
Jun 10, 2024

Conversation

ajeckmans
Copy link
Contributor

@ajeckmans ajeckmans commented May 30, 2024 •
edited
Loading

Updates the Reqnroll.Verify plugin to work with the latest Verify release 24.2.0.
With this change the plugin also does not need to be a generator plugin anymore, but just a runtime plugin.

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • Performance improvement
  • Refactoring (so no functional change)
  • Other (docs, build config, etc)

Checklist:

  • I've added tests for my code. (most of the time mandatory)
  • I have added an entry to the changelog. (mandatory)
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@ajeckmans ajeckmans marked this pull request as ready for review May 30, 2024 15:26
@ajeckmans ajeckmans self-assigned this May 30, 2024
@ajeckmans
Copy link
Contributor Author

@gasparnagy so we had this debate on what is a breaking change and what is not. Do I need to change the changelog to feature improvement or...?

@gasparnagy
Copy link
Contributor

@ajeckmans Please wait, I wanted to double check if my assumption really works.

gasparnagy
gasparnagy requested changes Jun 4, 2024
View reviewed changes
Copy link
Contributor

@gasparnagy gasparnagy left a comment

Choose a reason for hiding this comment

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

Please see my minor comments.

CHANGELOG.md Outdated Show resolved Hide resolved
....Verify.ReqnrollPlugin.IntegrationTest/Reqnroll.Verify.ReqnrollPlugin.IntegrationTest.csproj Outdated Show resolved Hide resolved
....Verify.ReqnrollPlugin.IntegrationTest/Reqnroll.Verify.ReqnrollPlugin.IntegrationTest.csproj Show resolved Hide resolved
Plugins/Reqnroll.Verify/Reqnroll.Verify.ReqnrollPlugin/Reqnroll.Verify.ReqnrollPlugin.csproj Outdated Show resolved Hide resolved
Plugins/Reqnroll.Verify/Reqnroll.Verify.ReqnrollPlugin/build/Reqnroll.Verify.targets Outdated Show resolved Hide resolved
Plugins/Reqnroll.Verify/Reqnroll.Verify.ReqnrollPlugin/build/Reqnroll.Verify.props Outdated Show resolved Hide resolved
@ajeckmans
Update verify dependency to latest version
d9f32f8
@ajeckmans
Removed adding UsesVerify Attribute as it was obsolete. As a result t…
a3c8b04 
…he generator was not needed anymore, so it is removed as well.
@ajeckmans
update changelog
f19c38f
@ajeckmans
Removed dependency on Microsoft.CSharp for Reqnroll.Verify.ReqnrollPl…
bccee47 
…ugin

Cleaned Reqnroll.Verify.ReqnrollPlugin.csproj to be more consistent with the other plugin project files
Fixed wrong tags in Reqnroll.Verify.nuspec
@ajeckmans
Changed a few things left form the generator to runtime plugin conver…
14ace1f 
…sion
@ajeckmans
Copy link
Contributor Author

@gasparnagy can you check again?

@ajeckmans ajeckmans requested a review from gasparnagy June 4, 2024 15:36
gasparnagy
gasparnagy requested changes Jun 5, 2024
View reviewed changes
Copy link
Contributor

@gasparnagy gasparnagy left a comment

Choose a reason for hiding this comment

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

Small changes requested. I haven't done a manual check this time, please make sure you try it.

Side note: For me it is better if you do not apply rebase or force push after an initial review has been done, because this way I have to make a full review again and I cannot just see the "diff". Merging the main to the PR branch is fine - we will squash it anyway.

Plugins/Reqnroll.Verify/Reqnroll.Verify.ReqnrollPlugin/Reqnroll.Verify.ReqnrollPlugin.csproj Outdated Show resolved Hide resolved
@ajeckmans
Copy link
Contributor Author

@gasparnagy I checked the local produced nuget package in an existing real world project and it is working fine.

@ajeckmans ajeckmans requested a review from gasparnagy June 5, 2024 17:51
gasparnagy
gasparnagy approved these changes Jun 10, 2024
View reviewed changes
Copy link
Contributor

@gasparnagy gasparnagy left a comment

Choose a reason for hiding this comment

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

Thx!

@gasparnagy gasparnagy merged commit dd7ecf6 into main Jun 10, 2024
4 checks passed
@gasparnagy gasparnagy deleted the update-verify branch June 10, 2024 08:05
@gasparnagy gasparnagy restored the update-verify branch June 10, 2024 08:23
gasparnagy added a commit that referenced this pull request Jun 10, 2024
@gasparnagy
Revert "Update verify plugin for Verify v24 (#151)"
0dbe22c 
This reverts commit dd7ecf6.
@gasparnagy
Copy link
Contributor

@ajeckmans Sorry, I forgot that we should not merge this before v2.0.3. I'm undoing it now.

@gasparnagy gasparnagy mentioned this pull request Jun 10, 2024
Closed
gasparnagy pushed a commit that referenced this pull request Jun 12, 2024
@ajeckmans @gasparnagy
Reqnroll.Verify: Support for Verify v24 (Verify.Xunit v24.2.0) for .N…
cac1020 
…ET 4.7.2+ and .NET 6.0+. (#151, #170)
gasparnagy added a commit that referenced this pull request Jun 12, 2024
@gasparnagy
Merge branch 'main' into testthreadid
0c31425 
* main:
  Reqnroll.Verify: Support for Verify v24 (Verify.Xunit v24.2.0) for .NET 4.7.2+ and .NET 6.0+. (#151, #170)
  use pull_request_template.md from the .github repo
  Fix spelling mistake in CONTRIBUTING.md
  Bump version
  fix config docs
  Add release contributors to CHANGELOG (#171)
  Update CONTRIBUTING with "draft PR" workflow and no force push after review
  fix CHANGELOG
  Fix project template binding (#169)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gasparnagy gasparnagy gasparnagy approved these changes

Assignees

@ajeckmans ajeckmans

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ajeckmans @gasparnagy