Release 0.2 2018

For my second open source release, I had a lot planned weeks in advance. However due to the nature of open source sometimes things don’t go as planned and you have to improvise. In my case, I had to find a completely new project to escape the plague of my pull requests getting closed consistently and as noted in my previous post I ended up finding a pretty good project called Chart.js which is a JavaScript library to generate detailed charts. The bug I picked up for the project was to fix the array tick label alignment. Example:

Problem:

p1.JPG

Solution:

ch2

As shown above, the main problem here is that the initial label in the array is centered however each consecutive label in the array is being printed below it and not being centered. To fix this initially I wrote:

context.translate(0, -tickFont.size * (label.length / 2));

which moves the labels up by the font size multiplied by the length of the array divided by two. This equation essentially finds the spacing between one label and the other and moves it up by that amount of space. For example to move “Row two” to the position of “Row one” it would need to be translated by negative eighteen. This was what I put up in my first pull request and as a result I got some good feedback.

p2.JPG

p3.JPG

It turns out my initial approach to fixing this issue had some flaws and thankfully I got some great guidance instead of having my pull request closed instantly like when I was working on other projects. Pettiness aside the problem with the code was that it was  slower to translate the label via the context than to move them using “y”. “y” in the code sets the initial printing position of the label so offsetting “y” would create an easier solution than translating the labels after the fact which is something I hadn’t thought of. Also if the graph was in vertical mode my code wouldn’t account for it and it ended up moving the labels into the bars.

Taking all the feedback into consideration I began writing code to update the pull request with the necessary changes.

Result:

p4

This code also had some minor problems like using ternary operator instead of an if statement to check if the graph is horizontal and using predefined variables in the for loop. I began working with Simon Brunel who is one of the lead developers for the project so we could both find the right equation. It was apparent to me that we both different ideas on how to write the equation and some which were both incorrect.

p5

p6.JPG

In the pictures above Simons comment is the result of my equation and my comment is the result when using his equation. We were both slightly off but I’m glad I had someone to help me with the process. From this point, it seemed like we were both awfully close and were missing the small thing that would make the equation work for all cases. Shortly after that comment, I decided to try to divide the whole equation by 1.2 which was the hardcoded line space value in the project. I thought this worked, however, Simon found the real reason the equation wasn’t working.p7

Simons new equation ended up working thankfully and I put up an updated PR that got approved and merged shortly after. Although it wasn’t me who figured out the correct equation, in the end, I still felt like a fundamental part of the development process. Also being new to the project I didn’t know the key terms and variables that would influence the creation of the correct equation. Overall I really enjoyed working on this project and hope to tackle other bugs in the future I loved all the feedback and help which will keep me coming back for more bugs!

Pull requests that were wanted features but were rejected for this release:

Adds Custom Error Message

Adds Next and Previous Song Buttons

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google+ photo

You are commenting using your Google+ account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

w

Connecting to %s