Skip to content

Remove git push commands after creating a new commit or branch #472

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

Merged
merged 7 commits into from
May 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

### Breaking Changes

_None_
- Remove git push commands after creating a new commit or branch. [#472] See `MIGRATION.md` for instructions.

### New Features

Expand Down
7 changes: 7 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Migration Instructions for Major Releases

### From `7.0.0` to `8.0.0`

We are no longer pushing to remote after creating a new commit or a branch. That means, developers need to manually push the changes or add push commands in the project's `Fastfile`. Most importantly, we can no longer immediately trigger beta/final builds after creating a new commit because the changes will not be in remote yet. If you want to keep the existing behavior, you'll need to add a push command before these triggers.

For example, in [WordPress-Android's `new_beta_release` lane](https://github.com/wordpress-mobile/WordPress-Android/blob/0c64cb84c256e004473e97d72b4ac6682ebc140b/fastlane/lanes/release.rb#L86), we download translations, bump the beta version and then trigger a new build in CI. After migrating to `8.0.0` of `release-toolkit`, we'll need to add [`push_to_git_remote`](https://docs.fastlane.tools/actions/push_to_git_remote/) command before this trigger to keep the existing behavior.
Copy link
Contributor

Choose a reason for hiding this comment

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

For reference, we discussed having a MIGRATION.md document during @iangmaia's trial, in #452

My understanding of the conclusion reached at the time was to put the information into the CHANGELOG.md to avoid having multiple file to keep track of and update.

It's a "tough" call. On the one hand, it's neat to move the info here to avoid cluttering the changelog. On the other, separating information might make it less discoverable. ⚖️

For what is worth, since you already implemented this I'm happy to keep it. It will be easy enough to move the content into CHANGELOG.md later on if we find it cumbersome or lossy to maintain both files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @mokagio! I missed this discussion.

I like having a separate file - as is probably clear from my proposal in the first place, but I am happy to move the instructions to CHANGELOG file. I just need some guidance on where exactly I should put it.

Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def self.run(params)
Action.sh('./gradlew', params[:lint_task])
end

Fastlane::Helper::GitHelper.commit(message: 'Update translations', files: res_dir, push: true) unless params[:skip_commit]
Fastlane::Helper::GitHelper.commit(message: 'Update translations', files: res_dir) unless params[:skip_commit]
end

#####################################################
Expand All @@ -38,7 +38,7 @@ def self.description
end

def self.details
'Download translations from GlotPress, update local strings.xml files accordingly, lint, commit the changes, and push to the remote'
'Download translations from GlotPress, update local strings.xml files accordingly, lint, commit the changes'
end

def self.available_options
Expand Down Expand Up @@ -90,7 +90,7 @@ def self.available_options
FastlaneCore::ConfigItem.new(
key: :skip_commit,
env_name: 'FL_DOWNLOAD_TRANSLATIONS_SKIP_COMMIT',
description: 'If set to true, will skip the commit/push step. Otherwise, it will commit the changes and push them (the default)',
description: 'If set to true, will skip the commit step',
type: Boolean,
default_value: false
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def self.run(params)
next_version = Fastlane::Helper::Android::VersionHelper.calc_next_release_short_version(params[:new_version])

Fastlane::Helper::ReleaseNotesHelper.add_new_section(path: path, section_title: next_version)
Fastlane::Helper::GitHelper.commit(message: "Release Notes: add new section for next version (#{next_version})", files: path, push: true)
Fastlane::Helper::GitHelper.commit(message: "Release Notes: add new section for next version (#{next_version})", files: path)

UI.message 'Done.'
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ def self.run(params)
repo_clean = repo_status.empty?
unless repo_clean
Action.sh('git commit -m "Update metadata strings"')
Action.sh('git push')
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def self.run(params)
next_version = Fastlane::Helper::Ios::VersionHelper.calc_next_release_version(params[:new_version])

Fastlane::Helper::ReleaseNotesHelper.add_new_section(path: path, section_title: next_version)
Fastlane::Helper::GitHelper.commit(message: "Release Notes: add new section for next version (#{next_version})", files: path, push: true)
Fastlane::Helper::GitHelper.commit(message: "Release Notes: add new section for next version (#{next_version})", files: path)

UI.message 'Done.'
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ module Android
# Helper methods to execute git-related operations that are specific to Android projects
#
module GitHelper
# Commit and push the files that are modified when we bump version numbers on an iOS project
# Commit the files that are modified when we bump version numbers on an Android project
#
# This typically commits and pushes the `build.gradle` file inside the project subfolder.
# This typically commits the `version.properties` inside root folder or `build.gradle` file
# inside the project subfolder.
#
# @env PROJECT_ROOT_FOLDER The path to the git root of the project
# @env PROJECT_NAME The name of the directory containing the project code (especially containing the `build.gradle` file)
Expand All @@ -16,14 +17,12 @@ def self.commit_version_bump
if File.exist?(Fastlane::Helper::Android::VersionHelper.version_properties_file)
Fastlane::Helper::GitHelper.commit(
message: 'Bump version number',
files: File.join(ENV['PROJECT_ROOT_FOLDER'], 'version.properties'),
push: true
files: File.join(ENV['PROJECT_ROOT_FOLDER'], 'version.properties')
)
else
Fastlane::Helper::GitHelper.commit(
message: 'Bump version number',
files: File.join(ENV['PROJECT_ROOT_FOLDER'], ENV['PROJECT_NAME'], 'build.gradle'),
push: true
files: File.join(ENV['PROJECT_ROOT_FOLDER'], ENV['PROJECT_NAME'], 'build.gradle')
)
end
end
Expand Down
13 changes: 4 additions & 9 deletions lib/fastlane/plugin/wpmreleasetoolkit/helper/git_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,39 +73,35 @@ def self.update_submodules
Action.sh('git', 'submodule', 'update', '--init', '--recursive')
end

# Create a new branch named `branch_name`, cutting it from branch/commit/tag `from`, and push it
# Create a new branch named `branch_name`, cutting it from branch/commit/tag `from`
#
# If the branch with that name already exists, it will instead switch to it and pull new commits.
#
# @param [String] branch_name The full name of the new branch to create, e.g "release/1.2"
# @param [String?] from The branch or tag from which to cut the branch from.
# If `nil`, will cut the new branch from the current commit. Otherwise, will checkout that commit/branch/tag before cutting the branch.
# @param [Bool] push If true, will also push the branch to `origin`, tracking the upstream branch with the local one.
#
def self.create_branch(branch_name, from: nil, push: true)
def self.create_branch(branch_name, from: nil)
if branch_exists?(branch_name)
UI.message("Branch #{branch_name} already exists. Skipping creation.")
Action.sh('git', 'checkout', branch_name)
Action.sh('git', 'pull', 'origin', branch_name)
else
Action.sh('git', 'checkout', from) unless from.nil?
Action.sh('git', 'checkout', '-b', branch_name)
Action.sh('git', 'push', '-u', 'origin', branch_name) if push
end
end

# `git add` the specified files (if any provided) then commit them using the provided message.
# Optionally, push the commit to the remote too.
#
# @param [String] message The commit message to use
# @param [String|Array<String>] files A file or array of files to git-add before creating the commit.
# Use `nil` or `[]` if you already added the files in a separate step and don't wan't this method to add any new file before commit.
# Also accepts the special symbol `:all` to add all the files (`git commit -a -m …`).
# @param [Bool] push If true, will `git push` to `origin` after the commit has been created. Defaults to `false`.
#
# @return [Bool] True if commit and push were successful, false if there was an issue during commit & push (most likely being "nothing to commit").
# @return [Bool] True if commit was successful, false if there was an issue (most likely being "nothing to commit").
#
def self.commit(message:, files: nil, push: false)
def self.commit(message:, files: nil)
files = [files] if files.is_a?(String)
args = []
if files == :all
Expand All @@ -115,7 +111,6 @@ def self.commit(message:, files: nil, push: false)
end
begin
Action.sh('git', 'commit', *args, '-m', message)
Action.sh('git', 'push', 'origin', 'HEAD') if push
return true
rescue
return false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ module Ios
# Helper methods to execute git-related operations that are specific to iOS projects
#
module GitHelper
# Commit and push the files that are modified when we bump version numbers on an iOS project
# Commit the files that are modified when we bump version numbers on an iOS project
#
# This typically commits and pushes:
# This typically commits:
# - The files in `./config/*` – especially `Version.*.xcconfig` files
# - The `fastlane/Deliverfile` file (which contains the `app_version` line)
# - The `<ProjectRoot>/<ProjectName>/Resources/AppStoreStrings.pot` file, containing a key for that version's release notes
Expand All @@ -25,10 +25,10 @@ def self.commit_version_bump(include_deliverfile: true, include_metadata: true)
files_list.append File.join(ENV['PROJECT_ROOT_FOLDER'], ENV['PROJECT_NAME'], 'Resources', ENV['APP_STORE_STRINGS_FILE_NAME'])
end

Fastlane::Helper::GitHelper.commit(message: 'Bump version number', files: files_list, push: true)
Fastlane::Helper::GitHelper.commit(message: 'Bump version number', files: files_list)
end

# Calls the `Scripts/localize.py` script in the project root folder and push the `*.strings` files
# Calls the `Scripts/localize.py` script in the project root folder and commit the `*.strings` files
#
# That script updates the `.strings` files with translations from GlotPress.
#
Expand All @@ -42,7 +42,7 @@ def self.commit_version_bump(include_deliverfile: true, include_metadata: true)
def self.localize_project
Action.sh("cd #{get_from_env!(key: 'PROJECT_ROOT_FOLDER')} && ./Scripts/localize.py")

Fastlane::Helper::GitHelper.commit(message: 'Update strings for localization', files: strings_files, push: true) || UI.message('No new strings, skipping commit')
Fastlane::Helper::GitHelper.commit(message: 'Update strings for localization', files: strings_files) || UI.message('No new strings, skipping commit')
end

# Call the `Scripts/update-translations.rb` then the `fastlane/download_metadata` Scripts from the host project folder
Expand All @@ -56,11 +56,11 @@ def self.localize_project
def self.update_metadata
Action.sh("cd #{get_from_env!(key: 'PROJECT_ROOT_FOLDER')} && ./Scripts/update-translations.rb")

Fastlane::Helper::GitHelper.commit(message: 'Update translations', files: strings_files, push: false)
Fastlane::Helper::GitHelper.commit(message: 'Update translations', files: strings_files)

Action.sh('cd fastlane && ./download_metadata.swift')

Fastlane::Helper::GitHelper.commit(message: 'Update metadata translations', files: './fastlane/metadata/', push: true)
Fastlane::Helper::GitHelper.commit(message: 'Update metadata translations', files: './fastlane/metadata/')
end

def self.strings_files
Expand Down
14 changes: 1 addition & 13 deletions spec/git_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
expect(Fastlane::Helper::GitHelper.has_git_lfs?).to be false
end

context('commit(message:, files:, push:)') do
context('commit(message:, files:)') do
before(:each) do
allow_fastlane_action_sh()
@message = 'Some commit message with spaces'
Expand Down Expand Up @@ -108,18 +108,6 @@
expect_shell_command('git', 'commit', '-a', '-m', @message)
Fastlane::Helper::GitHelper.commit(message: @message, files: :all)
end

it 'does not push to origin if not asked' do
expect_shell_command('git', 'commit', '-m', @message)
expect_shell_command('git', 'push', any_args).never
Fastlane::Helper::GitHelper.commit(message: @message)
end

it 'does push to origin if asked' do
expect_shell_command('git', 'commit', '-m', @message)
expect_shell_command('git', 'push', 'origin', 'HEAD').once
Fastlane::Helper::GitHelper.commit(message: @message, push: true)
end
end

describe '#is_ignored?' do
Expand Down