I was successful on making a pull request in the first week of October to fix one of the issues I blogged about in Lab 3 Blog.
What is the issue ?
The issue involved changing the directory structure of the app. This issue focused on moving and overhaul the CSS of page-list component. The real trouble would be to update test files because I have 0 knowledge of how test files work or what even what it does. To get started, I forked the repository (link to my fork)and then cloned it into my computer (Follow steps to learn forking and cloning in my Release 0.1 Blog). Once done, I went through the project structure and investigated the places where page-list component is being used so that when I move the component, I have to update the path of the file in other files that import this component. Once I found all the files, I proceeded to creating the new directory structure. The aim was to create a new directory named “page-list” which will contain files page-list.jsx and page-list.css.
Get To Fixing It
I moved the existing page-list.jsx to page-list directory and created a new file page-list.css which would contain css code from styles.css. At he same time I updated paths in the import statements in other components.
The directory structure looked like this after :
After this, I copied the css from styles.css file to page-list.css and edited the css selectors as was told to do in the description of the issue and added property styleName in tags in page-list.jsx according to the new CSS selectors.
After the fixes were done I created a new branch and committed them to the new branch.
git checkout -B issue-407 git add . git commit -m 'Fixes issues 407' git push origin issue-407
You can view the new branch issue-407 with new code here.
Creating the Pull Request
Now it was time to create the pull request. I went to my fork of the repository switched to branch issue-407 and pressed the New Pull Request button. It created a new pull request which you can view by clicking here.
Well, the pull request failed the tests ran by Circle CI. Circle CI is a tool used to run tests on pull requests automatically to determine the new code passs the test or not. The reason was simple. I changed the path to the page-list.jsx component everywhere the component was being imported but I forgot to change it in the test file. After reading the code in test file and reading through the errors I was getting, I changed the path of page-list.jsx in the test file and submitted a new commit. The pull request passed the Circle CI test and I waited for a review on my first ever PR.
The issue I fixed was in a project that had very active and helpful contributors. In the issue, the creator clearly described what to do and even helped me and gave me tips after I submitted a pull request as I messed up on my first attempt. Till then Happy Coding!
Update (October 3rd 2019)
So I checked my pull request and it was reviewed by one of the other leading project contributors and he asked me to improve some stuff in the code like indentation and some CSS selectors. As of October 4th, I committed a file with all those things done and updated the test file to support new CSS selectors. The updating test file was really hard this time so I had to lot of searching and read some documentation to understand the error I was getting and how to fix it. Finally, I managed to pass the test locally using npm test command and submitted a commit. You can view all the conversation about my code and issues with it at : https://github.com/edgi-govdata-archiving/web-monitoring-ui/pull/414
- Issue Link : https://github.com/edgi-govdata-archiving/web-monitoring-ui/issues/407
- Repository Link: https://github.com/edgi-govdata-archiving/web-monitoring-ui
- My Fork Link: https://github.com/jatinAroraGit/web-monitoring-ui/tree/master
- Branch with new code to fix the issue : https://github.com/jatinAroraGit/web-monitoring-ui/tree/issue-407
- Pull Request Link: https://github.com/edgi-govdata-archiving/web-monitoring-ui/pull/414