Firefox for iOS is the mobile version of the popular Mozilla Firefox web browser designed specifically for Apple's iOS operating system. I'm very glad to have contributed to firefox-ios. I would like to share my PR process. Hope it's helpful to you.
Issue
The issue was opened by data-sync-user. This issue was about the refactor request that replace previous button with the new designed PrimaryRoundedButton. In this issue, we can also see a reference PR by lmarceau from Mozilla. That was really helpful for me to start.
Refactor process
Prepare working environment
Walk through the official Building the code process. I want to mention two things of the steps:
sh ./bootstrap.sh
I met the following error when running this command on my M1 macbook air.
xcrun: error: unable to lookup item 'PlatformPath' from command line tools installation
xcrun: error: unable to lookup item 'PlatformPath' in SDK '/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk'
I fixed this by: (Solution provided by l'L'l )
$ sudo xcode-select -switch /Applications/Xcode.app/Contents/Developer
Make sure to select the Fennec scheme in Xcode.
This means setting scheme from where the blue box shown in below screenshot:
Others should be all provided in the repo's README.md.
Learn from the reference PR and Refactor
I learned from the reference PR how to refactor. It was like updating the definition then remove unused variables. For example:
Before:
private lazy var mainButton: ResizableButton = .build { button in
button.contentHorizontalAlignment = .center
button.buttonEdgeSpacing = 0
button.layer.cornerRadius = UX.mainButtonCornerRadius
button.contentEdgeInsets = UX.mainButtonInsets
button.addTarget(self, action: #selector(self.didTapMainButton), for: .touchUpInside)
}
After:
private lazy var mainButton: PrimaryRoundedButton = .build { button in
button.addTarget(self, action: #selector(self.didTapMainButton), for: .touchUpInside)
}
You can see now we don't need the contentHorizontalAlignment and buttonEdgeSpacing settings. This is because PrimaryRoundedButton has already defined.
Conflicts and Rebase
When the review was going on, I got a notation that informed my pr has conflicts when rebasing.
This means the main branch has updated and my PR can't merge to the main branch due to the conflicts. Here is how I did to resolve:
1. Sync fork
On the forked repo, click the Sync fork button and let Github do for you to keep the forked repo's main branch up to date with mozilla-mobile/firefox-ios:main
2. Update local repository
git checkout main
git pull origin main
3. switch to the branch where have made the changes, here mine is issue-16864
git checkout issue-16864
4. Rebase
git rebase main
Here we need to resolve conflicts manually, I used VS Code located my modified files and it could let me select which code to use.
5. Check status
After resolving conflicts, I can see the following info from git status
On branch issue-16864 Your branch and 'origin/issue-16864' have diverged, and have 26 and 1 different commits each, respectively. (use "git pull" to merge the remote branch into yours)
nothing to commit, working tree clean
The git status message indicates that my local branch and the remote branch have diverged due to the rebase, but there are no uncommitted changes in my working directory. This is expected after a successful rebase.
Note that although it shows
use "git pull" to merge the remote branch into yours
We don't need to do that.
Also note that on the VS Code SOURCE CONTROL Tab, it shows me a button with Sync Changes 1 down and 26 UP. This is a shortcut that performs both a git pull and a git push operation. The numbers (1 down, 26 up) represent the number of commits that are different between my local branch and the remote branch. "1 down" means there is 1 commit on the remote branch that I don't have locally, and "26 up" means there are 26 commits locally that are not on the remote branch.
However, in my case, I should not use the "Sync Changes" button. This is because I've performed a git rebase, which has rewritten the commit history of my local branch. If I were to sync now, it would try to merge the divergent histories of the local and remote branches, which is not what I want.
6. Force Push
git push origin issue-16864 --force
What we do after rebase is force push the my local branch issue-16864 to the remote repository to overwrite the remote branch with my local branch.
A little More
After force push, I can see in the PR page, there is a from commit-a to commit-b and a compare link
Follow the link, I can see many changes compare to the base: main
The "compare" link on GitHub shows the differences between the two points in history I'm comparing. This means that the comparison will include all changes that happened on the main branch since the point where my branch originally diverged from it. On the other hand, the "Files changed" tab in the pull request only shows the changes I made in my branch.
If my pull request is accepted and merged, only the changes I made in my branch (the ones shown in the "Files changed" tab of the pull request) will be merged into the main branch.
The other changes that I see when I click the "compare" link are changes that have already been made in the main branch. These changes are already in the main branch, so they won't be affected by my pull request.
Reflection
This contributing process has allowed me to learn more about the Git workflow and the concepts of controllers and view models, which has been a valuable learning experience. I even chatted with lmarceau from the Element channel #fx-ios and appreciate the help I've received from the community. I'm excited to continue my journey and further contribute to this project. Looking forward to seeing you there!
Top comments (0)