Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed library to be OOP #79

Closed
wants to merge 2 commits into from
Closed

Changed library to be OOP #79

wants to merge 2 commits into from

Conversation

codewithtyler
Copy link
Contributor

Not only is the library now completely object-oriented, but I've also added JSCS and updated JSHint so that the library now has a consistent code style.

This can be changed later if needed, but the code style I chose to use is the jQuery Style Guide. I chose it because we were already following most of it's guidelines. I did; however, have to make a couple necessary changes to this style.

  1. I updated the maximumLineLength to be 175 characters. I did this because we have several if statements that exceed this limit. Some of our functions also have several parameters which makes the function declaration rather long. In the jQuery Style Guide a line can only be 100 characters and setting this to 175 gets around that limit with our long lines.
  2. Last, I had to update the validateLineBreaks to allow for CRLF. This is due to the way Git handles line breaks.

Special Thanks Goes Out To:

  • @johnkpaul - for helping me understand more about developing an OOP library
  • @andymac-2 - for helping me work out the last few bugs in the library

@codewithtyler
Copy link
Contributor Author

@steveathon because of how much code was changed, what do you think about increasing the version number to 1.1 instead of 1.0.6?

@andymac-2
Copy link

In response to maximum line length, if statements and function calls can be separated onto multiple lines, as per JQuery guidelines, It is a good rule to have short line lengths, as some people have thin screens, large text or like two text files abreast.

@DerDu
Copy link

DerDu commented Jan 8, 2016

+1 Sweet!

@johnkpaul
Copy link

So glad to have seen this come along so much since we chatted first a few months ago. Great job @RandomlyKnighted!

@steveathon
Copy link
Owner

This is awesome - we're re-targeting to 2.0-dev because it's a big change so we can test it out a bit before we merge it completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants