Printable View
and just that quick I think I figured it out.. (would love someone to grade my homework though) I created a fork and submitted a change as follows:Quote:
Originally Posted by DivaVocals
Quote:
Replace jQuery with the latest jQuery version.
Pulls latest jQuery from CDN.
So, looks like you successfully have updated github to reflect the changes that you have made, which went a step further than provided by Design75 by deleting what looks like the contents of the the jquery file (haven't downloaded the applicable files to see if actually the file itself and associated folders were deleted or just the contents of the file), but then kept a reference to the internet version of the jscript_jquery file. So couple of comments based on a review and I wouldn't call myself a grading teacher:Quote:
Originally Posted by DivaVocals
Don't see a pull request from niestudio's version (so, yes forked, but no notification back to the main project to request merging).
It looks like the project was forked from niestudio's version which does not yet include Design75's changes.
The changes made in both forks are different, as well as similar, this would result in niestudio having to decide/determine a "best" path forward as to which to incorporate and will likely require a manual merge, but that is the way of working in/with github.
Next homework assignment is to provide a new commit to your own branch, create two branches off of yours, make a modification in each branch that is in two different areas, merge the two branches, submit a pull request for your changes, let's see what else? :)
:laugh: I'll have to read and try and digest this all after I eat.. I already submitted the pull request for my changes.. (figured out that I missed this all on my own..:smile: just need someone to tell me if I did it "right":smile:)Quote:
Originally Posted by mc12345678
In summary my changes included deleting the outdated v1.4.1 jQuery file that is included with Zen Lightbox in it's entirety, and replacing it with a NEW file that instead pulls the latest version of jQuery from a CDN.. (I followed the same pattern that Picaflor follows in her templates)Quote:
Originally Posted by mc12345678
Now going forward should I have simply worked from Design75's repo or niestudios's original repo versus creating my own fork???.. As I said I know what gihub is for, but have never used it in practice.. So I simply followed Design75's lead and created my own fork.. If this was the wrong path, then I would appreciate some tips on best practices here..
Quote:
Originally Posted by mc12345678
I like that "cliff's notes" version of github collaboration. :)
While I do agree that jQuery probably doesn't need to be loaded in the way that it has with this plugin since the beginning.
With these new proposals, my concern with pulling latest jQuery from the cdn is that ZLB is based on slimbox which is not really actively developed anymore. There was an update in May (that has not been applied to ZLB) to address jQuery 1.9 compatibility, but the previous one was 3+yrs prior.
So, had we been pulling latest jQuery from CDN all this time, it may have broken everyones ZLB until there was an update to slimbox.
Also, shouldn't we have a fallback to check if !jQuery {load jquery} incase the user has not used a template or other plugin that already has jQuery?
any thoughts on this?
The fallback option was a thought that I had, as well as the concerns of always loading/forcing the latest. Yes programs should always seek to use the latest, and if everything else is kept up-to-date, going out and calling the latest is probably a great way to go. I did at one time (2 years ago) collaborate on a project where they were calling the latest from the internet, and the resulting delay was actually causing trouble as compared to simply loading it locally, but at the same time can understand the reasons for not loading it again, and again, and again.... (almost worse if all of them are using different versions).Quote:
Originally Posted by niestudio
I agree with the fallback code.. it's a BIG problem with sooooo many modules and templates using jQuery and shopowners unwittingly having MULTIPLE versions of jQuery running in their shops.. (which was the inspiration for my contribution)Quote:
Originally Posted by niestudio
Couldn't backwards compatibility be mitigated with the jQuery Migrate script??
Code:<script src="http://code.jquery.com/jquery-migrate-1.2.1.js"></script>
If you and Design75 are working on the same "goal" then probably better to repo/fork off of Design75's version so that the changes made by the two of you could be merged together by one of you and then brought into niestudio's master. If you each are working on something different, like each working on a different feature, or "black box", then repo'ing off of a functional path would be best/easiest. In short, the collaborative effort actually requires collaboration, not just at the software level (which allows "checking out" each other's work), but also in the communication of what is being done to minimize rework and duplication of effort.Quote:
Originally Posted by DivaVocals
Also, by forking off the master, the master can be changed, including building more functionality, and eventually the or one of the forks can be merged with the master (possibly to even create a new branch) to verify downstream/future compatibility, and then could be rewound also to find in which commit caused a conflict.
I personally thought it was great to see you run off and give it a shot. Unfortunately you and one other basically did the same thing, so the result didn't give you the best first experience. :)
In short, usually best to pick a "known" point that is good/works (which you can do before actually forking the project) and then work from that point. When others provide input, always possible to provide comment associated with what they have shared with github/the thread "runner". (That person also can add others to help manage the master thread.) Also, there are ways/places to provide input about the mod as part of pushing each commit, and depending on your interface software this may be "clearer" than with others, but generally the title will wrap around to the body of the statement(s), also things pushed to github, well... are out there and can not really be taken back. There are several readme files in github that are informative. Perhaps niestudio may have other hints/tips?
Quote:
Originally Posted by DivaVocals
Have you ever suggested that ZC roll a latest jQuery call into the ZC default template?
Hmm, I've never come across this before. Sounds interesting but would not have worked for this since it only supports jQuery 1.6.4 and later.
Going forward it could be good though.
I believe that v1.6 (though I haven't looked at the gibhub lately) is going to do JUST THAT.. Then there would be no NEED for plugin authors to have to include jQuery files which would also eliminate the issue with multiple jQuery versions being loaded on a shop..Quote:
Originally Posted by niestudio
It's a path WordPress also took in recent versions so that plugin developers wouldn't have to include jQuery in their plugins. Eliminates the number of plugin clashes due to conflicting versions of jQuery loading at once on a site..
Well I've used it with Zen Lightbox with no issues.. let me find the site and post the URL.. Here's the gihub for the jQuery Migrate script: https://github.com/jquery/jquery-migrate/#readmeQuote:
Originally Posted by niestudio
Edited to add: http://laserdiscvault(dot)com/
CDN calls for jQuery and the jQuery Migrate script.. This is both the changes Design75 and I submitted to github with the addition of the jQuery Migrate script.. Lightbox works with no issues..
haha , you make one little change in code and get almost 2,5 pages of reading in return :laugh:.
The reason I did not change the jquery version, is because I wanted to make this first change as easy as possible. I know ZLB runs fine on JQ 1.8.4 , and probably later versions. My knowledge of Jquery is only limited. I wanted to integrate the latest version of slimbox, but since I do not have enough knowledge of Jquery, and a limited amount of time, I choose to only remove the echo command and rename the script-name to start with jscript_ .