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

$variable naming & comments #725

Merged
merged 9 commits into from Jan 15, 2018
Merged

$variable naming & comments #725

merged 9 commits into from Jan 15, 2018

Conversation

SteveTheTechie
Copy link
Contributor

Mostly a variable re-naming and commenting update.

  1. Updated version number to 3.0.0 in code heading

  2. Updated jQuery version dependency to 1.7+ due to deprecation of $().delegate & $().bind(), and use of $().on() instead

  3. Detailed comments for each option to explain the permitted value types and the intended use of the option.

  4. Switch default for selectedText option to be # of # selected to ensure that developers know that this form is supported.

  5. Follow the popular convention of prefixing variable names of jQuery objects with a dollar sign for more clarity.

  6. Use local variables for properties fetched more than once.

  7. Ditch use of $().bind() in favor of $().on(), as bind is deprecated.

Unit tests have been updated.

Mostly changing to $naming scheme for jQuery objects and adding more comments.
bind() is deprecated.  Use on() instead.
Changed the default selectedText to be "# of # selected" so that developers will be aware that this form is available.  (It always has been available..)
@SteveTheTechie
Copy link
Contributor Author

SteveTheTechie commented Jan 13, 2018

I guess GitHub will not let me do a squash PR if I am bringing over a feature branch I created. ?

Sorry, I intended to do a squash commit. Thank you for your generous patience w/ me.

@mlh758
Copy link
Collaborator

mlh758 commented Jan 15, 2018

To squash it and have it reflected on the PR you would rebase and then force push the branch. The changes would be reflected in the PR automatically.

@mlh758 mlh758 merged commit 1676043 into ehynds:version3 Jan 15, 2018
@SteveTheTechie
Copy link
Contributor Author

Thanks. I wish I had your skill with Git and GitHub. I have done several online tutorials and read a lot about Git, but I still struggle w/ it.

@mlh758
Copy link
Collaborator

mlh758 commented Jan 16, 2018

I've just been using it for a few years. Have you used any version control software other than git, like SVN?

@SteveTheTechie
Copy link
Contributor Author

As I am a hobbiest developer (just do this in my spare time), I just do not have the experience with it. I have done a little bit with the check out/check in type of version control systems in the past (I think this is the SVN model).... most would be considered outdated/obsolete by today's standards.

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.

None yet

2 participants