DEV Community

Tue
Tue

Posted on

A way to review a Pull Request

This week's work consists of contributing to hacktoberfest repo and adding a new feature to a ssg repo which I am going to share my experience.

Adding code to support new feature

After looking at a few repos, I decided to filed an issue on Roxanne's repo.

It took me not too long to finish implementing the feature, Roxanne's code was broken into descriptive functions, therefore, it was a breeze to read.

The script has a few if statements to catch options like --input, --stylesheet, so my approach is to catch the --config option first and override all properties of the options object.

if(options.config !== undefined) {
    try {
        let configData = fs.readFileSync(options.config);
        let configOptions = JSON.parse(configData); 
        for(const [key, value] of Object.entries(configOptions)) {
            value || value.length > 0 ? options[`${key}`] = `${value}` : options[`${key}`] = undefined;
        }
        if(!options.input) {
            console.error(`error: input <file or directory> is not specified in config file ${options.config}`);
            process.exit(-1);
        }
    } catch(error) {
        console.error(`Can't read or parse config file ${options.config}\n ${error}`);
        process.exit(-1);
    }
}
Enter fullscreen mode Exit fullscreen mode

And if input option is missing from the config file which is required, the program will log the error and exit.

Testing and reviewing other's work using git remote

image

http://jlord.us/git-it/assets/imgs/remotes.png

Git remote makes it easier to test and review any pull request or additional code.

First, I added Roxanne's code as a remote to my local repo and fetch instead of pull to avoid merging to my local repo.

git remote add rclee1 https://github.com/rclee91/tue-1st-ssg.git
git fetch rclee1
Enter fullscreen mode Exit fullscreen mode

I then created a tracking branch which is a reference to Roxanne's fork branch issue-19 using

git checkout -b rclee1-issue-19 <rclee1>/<issue-19>
Enter fullscreen mode Exit fullscreen mode

In this branch, I experimented the script to check if there was anything to improve or fix and with this approach, reviewing a PR is definitely more effective than checking a PR on GitHub.

You can take a look at Roxanne's repo here or mine

Top comments (0)