[in_app_purchase_storekit] Fixup iOS integration test placeholder and remove exclusion#6604
Conversation
…rekit. The removed exclusion is no longer necessary after closing flutter/flutter#81695. See also [this comment](flutter#6517 (review)) where the obsolete exclusion surfaced.
|
I'm unsure if additional tests are necessary for this PR. As far as I can tell, removing the exclusion should already result in additional tests to be run. |
| @@ -1,3 +1,7 @@ | |||
| ## NEXT | |||
There was a problem hiding this comment.
Is this supposed to be a version number? What is NEXT?
There was a problem hiding this comment.
I used the update-release-info tool with the --version=minimal option.
This generated the NEXT automatically, indicating that the change does not required a version update.
But I guess from reading the docs, there shouldn't be an entry to the CHANGELOG at all, since this PR only affects development of the package, i.e. is a test-only change.
All version changes must have an accompanying CHANGELOG update. Even version-exempt changes should usually update CHANGELOG by adding a special NEXT entry at the top of CHANGELOG.md (unless they only affect development of the package, such as test-only changes):
How would you interpret this? Should I remove the CHANGELOG entry again, leave it as is, or add a proper version change?
There was a problem hiding this comment.
This is a tool bug; it wasn't updated when the repo policy changed (filed flutter/flutter#113941). You can just just remove the entry entirely, yes.
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
LGTM (ideally with the changelog removed, but it's fine either way). Thanks for cleaning this up!
(@a-wallen Was your comment meant to have the approval bit?)
| @@ -1,3 +1,7 @@ | |||
| ## NEXT | |||
There was a problem hiding this comment.
This is a tool bug; it wasn't updated when the repo policy changed (filed flutter/flutter#113941). You can just just remove the entry entirely, yes.
|
@stuartmorgan no, I never meant to give the green light. The patch LGTM, but I can't say I have enough experience with this topic to give an official LGTM. |
|
A conflict had developed in the CHANGELOG so I just resolved it via GitHub's resolution UI by removing the changes per discussion above. |
stuartmorgan-g
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
|
auto label is removed for flutter/plugins, pr: 6604, due to - The status or check suite repo_checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
stuartmorgan-g
added
the
override: no changelog needed
|
thx for removing the CHANGELOG entry, I haven't had time to revert the changes myself |
|
Looks like we need to add integration tests to the dev-only check for changelog validation. For now, adding the override label since it's a false positive. |
stuartmorgan-g
added
the
autosubmit
…holder and remove exclusion (flutter/plugins#6604)
… remove exclusion (flutter#6604)
… remove exclusion (flutter#6604)
4 participants
The removed exclusion is no longer necessary after closing flutter/flutter#81695.
See also this comment where the obsolete exclusion surfaced.
Closes flutter/flutter#113912
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.