Monday, September 23, 2019

Release 0.1

Release 0.1

This was a fun one, building upon our Lab2 we were to review other students' apps and make:
  • 1 bug fix
  • 1 enhancement
The release was great practice, because it provided us an idea of the process of contributing to an open source project.

  1. Reviewing code to create issues/review issues
  2. Fork the repo to your own repository
  3. Clone repository to local machine
  4. Create a working branch for the issue
  5. Make enhancement/Fix bugs
  6. Commit changes to branch
  7. Push branch to repository and make a pull request to master branch of the original repository
I worked on the following:

1https://github.com/evlnyng/CornellNote/issues/2
For this one I made an enhancement to include a package that would allow for hotkeys to be used while in the app to enable saving of all notes with ctrl+q. I previously had thought about creating a hotkey to close the window too, but it was redundant as users of windows/mac already have a key function for that. 

To allow this feature, I also had to allow create a save function as the existing save function in the code was written into one feature as part of the auto save. 

Furthermore, learning from my own mistake I made with my hotkey implementation on my own app, I decided to add message notifying the user, the current text has been saved.


In the future if I was to make a suggestion, I would suggest to rewrite the auto-save function to include my save function and make it more modular.


2. https://github.com/RyanWils/my-note/issues/7
This one was interesting, I learned a few things. Test, test, test do not assume. This was supposed to be a simple bug fix, the user had repeated the line document.querySelector('#note').innerHTML 5-6 times in their code. I thought great, let's suggest a variable to hold the whole thing to reduce the number of keystrokes in the future if the user decides to use the selector in the future. I stupidly assumed I could do the following:

  var noteHTMLSelector = document.querySelector('#note').innerHTML

and didn't follow up by testing to see if it broke or worked. Thankfully another contributor called Jacob caught the issue. I learned you can't assign the innerHTML part to a variable as strings are immutable in javascript (thank you stackoverflow).

In the end the following changes I made was:


  var noteSelector = document.querySelector('#note')


and users will have to use


  noteSelector.innerHTML


to access the text to make changes or to use for functions.


I received the following pull requests:

Again I learned a lesson from this one which I'll go into detail at the end. Review was a simple one, in my original code, anytime the app is saved: a message will be output to console that the note has been saved. However, unless you're a developer, most people don't check console on their browser, so they'll never see the message. This change allowed users to see explicitly see within the app, the note was saved. I also learned, potential enhancements/bug fixes can actually cause more bugs if they're not reviewed properly. Again, I should've paid more attention in reviewing the enhancement as it created an issue where by default a message showing the current note was saved even though nothing was typed, which paved the way for pull request #2

This removed the default save message displaying at the start. Simple fix, but made the app more clean.

On a side note, I had the biggest issue while making changes to repo 2 in the blog. I had set my upstream and everything to the remote branch, however things weren't updating... even with a git pull. When I pushed my changes and requested a pull from user, it wasn't being accepted because mine was out of date (or so I suspect). It took forever to figure out how to reset my local master to the version of remote master (I promised our prof I would only clone a repo once), after all that my master branch was behind my issue branch and it took more to figure out how to get the two to match up. I finally figured it out and that felt like an assignment in itself.

No comments:

Post a Comment

Async Await and Promises

As a continuation of my PR for Telescope, I thought I should talk a bit about async/await and the old way of using return new Promise(). Her...