-
Notifications
You must be signed in to change notification settings - Fork 29k
Update pubspec.yaml.tmpl build version tracker on line 5 #173600
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
base: master
Are you sure you want to change the base?
Conversation
…sistency between builds on IOS and Android. Created tests to accomodate.
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.
Code Review
This pull request aims to standardize the default build version for new Flutter projects to ensure consistency between iOS and Android by adding a build number to the pubspec.yaml.tmpl
for empty applications. My review has two main points: the version in pubspec.yaml.tmpl
should be 0.1.0+1
instead of 0.1.0+
to correctly set the default build number to 1, and the new test case should be adjusted to verify the behavior for an empty application (--empty
), as the current test covers a standard application, which is not affected by this change.
@@ -2,7 +2,7 @@ name: {{projectName}} | |||
description: {{description}} | |||
{{#withEmptyMain}} | |||
publish_to: 'none' | |||
version: 0.1.0 | |||
version: 0.1.0+ |
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.
The PR description states that the goal is to add a default +1
build number for consistency. However, the change to 0.1.0+
results in an empty build number, which doesn't resolve the inconsistency between Android and iOS platforms. To ensure both platforms default to a build number of 1, this should be 0.1.0+1
.
version: 0.1.0+1
testUsingContext('generated pubspec uses default build number (+1) for standard app', () async { | ||
await projectDir.create(recursive: true); | ||
|
||
await _createProject( | ||
projectDir, | ||
<String>['--no-pub', '--template=app'], | ||
<String>['lib/main.dart', 'pubspec.yaml'], | ||
); | ||
|
||
final String pubspec = await projectDir.childFile('pubspec.yaml').readAsString(); | ||
expect(pubspec, contains(RegExp(r'^version:\s*1\.0\.0\+1\s*$', multiLine: true))); | ||
}); |
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.
This test verifies the version for a standard app, which already had a build number and is not affected by this PR's changes. The change in pubspec.yaml.tmpl
is for an empty app (--empty
). The test should be updated to validate the version for an empty app to ensure the change is correctly tested.
testUsingContext('generated pubspec uses default build number (+1) for empty app', () async {
await projectDir.create(recursive: true);
await _createProject(
projectDir,
<String>['--no-pub', '--template=app', '--empty'],
<String>['lib/main.dart', 'pubspec.yaml'],
);
final String pubspec = await projectDir.childFile('pubspec.yaml').readAsString();
expect(pubspec, contains(RegExp(r'^version:\s*0\.1\.0\+1\s*$', multiLine: true)));
});
An existing Git SHA, To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with |
@@ -4838,6 +4838,19 @@ To keep the default AGP version $templateAndroidGradlePluginVersion, download a | |||
}, getCurrentDirectory: () => out); | |||
expect(logger.statusText, isNot(contains(r' $ cd'))); | |||
}, overrides: {Logger: () => logger}); | |||
|
|||
testUsingContext('generated pubspec uses default build number (+1) for empty app', () async { |
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.
We might want to add a test that also ensures the build number is present when using the default template (e.g. without the --empty
flag).
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.
I believe I tried running "Create Flutter test-app" withouth --empty, and it did not show up. What showed up instead was the other version number. If I understand correctly, it creates different templates with different flags, and this version number that I created a PR for specifically was for the --empty flag. The other version number already had a +1, and I was creating a pull request to make it more consistent.
The following defines the version and build number for your application. A version number is three numbers separated by dots, like 1.2.43 followed by an optional build number separated by a +. Both the version and the builder number may be overridden in flutter build by specifying --build-name and --build-number, respectively. In Android, build-name is used as versionName while build-number used as versionCode. Read more about Android versioning at https://developer.android.com/studio/publish/versioning In iOS, build-name is used as CFBundleShortVersionString while build-number is used as CFBundleVersion. Read more about iOS versioning at https://developer.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html In Windows, build-name is used as the major, minor, and patch parts of the product and file versions while build-number is used as the build suffix. version: 1.0.0+1 {{/withPlatformChannelPluginHook}}
P.S. I apolgize, I'm not good with Github text formatting
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.
Yup understood!
I just meant if there wasn't already an existing test we should add one to make sure the default template doesn't get out of sync in the future.
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.
I checked for that when making the PR, but I wasn't able to find one. But, if anyone does, that would be great!
Added a default +1 build number to the version field in pubspec.yaml.tmpl (line 5). This ensures consistent versioning behavior across iOS and Android for newly created Flutter projects.
Why this change?
Benefits
Developers can still override --build-name and --build-number as needed for custom versioning.
List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Fixes #165325
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assist
bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.