openpsa / jsgrid

Fork of last jqGrid version before license change
http://openpsa.github.io/jsgrid/
Other
28 stars 12 forks source link

Merging Oleg's changes up to the first January into master #27

Closed meh-uk closed 9 years ago

meh-uk commented 9 years ago

Just a housekeeping pull so we know what we've actually merged.

bouks commented 9 years ago

There's too much changes to "test" it. So i suppose we must be confident in Oleg's changes and that you have well applied them.

Is everybody ok or not ?

meh-uk commented 9 years ago

As you can see from the diff I haven't actually made any changes that @flack hadn't already cherrypicked :).

There's still the CSS to copy over.

bouks commented 9 years ago

Matt have you implemented it in a living project just a little seeing ?

Excuse me, i speak english but not this far :) ,what is "cherrypicked" ?

flack commented 9 years ago

@meh-uk I must admit I don't really understand this PR. Did you merge Oleg's commits and then delete the files we don't use? If so, what exactly is the benefit of merging in the first place, then?

About the CSS changes: Like I wrote earlier, we need to decide if we want to go with Less (I would be in favor of that, but I'll yield to the majority). If we do, we could either port Oleg's CSS changes manually, or we could re-do the less files based on his most recent CSS file. We only have to make sure that the CSS changes we integrate are in sync with the JS changes (e.g. Oleg removed some things from the HTML construction like celpadding and added them as CSS instead, so we have to pull both changes at the same time)

@bouks cherry-pick is a Git command. It allows you to merge individual commits from one branch into another. See http://git-scm.com/docs/git-cherry-pick

meh-uk commented 9 years ago

@flack it made things simpler in my head :). I'm not fussed if this isn't done though if you guys don't think it is helpful.

flack commented 9 years ago

I wonder what would happen if we merge this and then run (for example)

git checkout 48c38fbf363b7505d373f5843b104d78ba2528c8

I guess it would create the CSS file in your working directory, which might be confusing (and the other question would be if git checkout HEAD would remove it again. I guess it should, but I haven't tried it yet)

meh-uk commented 9 years ago

git checkout HEAD does nothing for me, but git checkout master then removes the css folder again.

meh-uk commented 9 years ago

Assigned to 1.0 to make a decision as to whether we accept or reject this as I foolishly did it on my 'master' branch.

meh-uk commented 9 years ago

OK its clear we don't want to do this, closing so I can have my master back!