This week, I added another feature to my classmate Ali's TIL Post to HTML conversion app, TIL-Markdown2. My classmate Yumei also added the same feature to my app, go-go-web.
The new feature is to implement support for config files (.toml) so users could specify flags and arguments in the config file rather than on the command line.
Making a PR on Ali's Repo
First I created issue-3 in my classmate's repo detailing the new feature.
Next I forked and cloned his repo. I created a new topic branch. I reviewed the code and tested the app and I found that the conversion for single file works well, but it was missing the conversion for folders. However, this was good enough for me to begin working on implementing the config file feature for single file conversion.
I found a open source TOML implementation, tomlkit, that was recommended to me by Yumei. I wrote a new function that uses tomlkit to parse the config file into a dictionary-like object. I made an initial commit to my branch.
After my initial commit, I made a draft pull request on the main repo. As I kept working on the feature implementation and making commits, I would go update this draft pull request with additional details of what I updated. When the feature was fully implemented and tested, I made the PR ready for review. I also contacted my classmate to let him know, along with some details of how to test the new feature.
UPDATE: My PR was later merged into the main repo with no issues :)
Reviewing Yumei's PR on My Repo
I have reviewed a PR on my repo before, but this time I used git remote to do it. I opened a terminal in VSCode and ran the following commands to get Yumei's PR code:
$ git remote add yumei https://github.com/WangGithub0/go-go-web.git
$ git fetch yumei
$ git checkout -b issue-10 yumei/issue-10
I reviewed and tested the code. The first thing I noticed was that the program did not run, since I needed to install a library. I ran the pip install tomlkit command and also asked Yumei to add this to the installation instructions in the README file. The new feature works well, but I noticed a few more things I wanted her to change in the README file. I made comments to code I wanted changed and changed the PR to "Changes Requested" status.
She made the fixes very quickly and re-requested a review on the PR. I then did a git pull to update my local tracking branch to see her changes. The changes looked good to me so I merged the pull request.
I learned a lot this week, including using an Open Source library in my code, and about using git remote and git branches. Using git remote was easier than I expected, and I did not run into any problems. In fact, it was quite easy to update my local tracking branch whenever Yumei made a change that I wanted. I will continue to use this method in the future to review future PRs. Next time, I will also try to use the git commands on the Terminal to merge the PR rather than the Merge PR button.
Top comments (2)
Thanks for sharing your experience, sounds like you are all getting proper hands-on with your tools 😊
I tend to only keep a single local repo per remote/fork unless i am in the process of actively transvasing, the mixing gets confusing over months... so i am pretty impressed to see you are managing your teammates remotes in your working repo. 💪
Side note, you might want to make it an early-learned habit to always install in virtualenvs when working with python, it will avoid lots of dependency knots in the future ... i learned the hard way 😭
thank you! I will check out virtualenv!