search

stampit-org / stamp-specification

The Stamp Specification: Composables
434 stars 16 forks source link

New merge algorithm #105

Closed koresar closed 8 years ago

koresar commented 8 years ago

The PR looks large, although there is only one actual change - the new merging algorithm. It implements #101

Minor changes:

danielkcz commented 8 years ago

Isn't there already some consensus to actually remove code from the specification repo? I believe @ericelliott has agreed on that too. The stampit has already side tracked little with separating merge functions in a different files Do you plan to copy&paste this back there?

koresar commented 8 years ago

@FredyC

Do you plan to copy&paste this back there?

Yes. I already did it in my local git repo.

danielkcz commented 8 years ago

@koresar Ok but still I don't see a reason why to keep the code in here. Check the comment here: https://github.com/stampit-org/stampit/issues/221#issuecomment-231564585

koresar commented 8 years ago

@FredyC I'm not seeing what you mean. This code is an example implementation.

Someday other people would like to make their own implementation of stamps. This repo will give them more understanding of what, and why, and how. That's basically it.

koresar commented 8 years ago

I'm merging this in few hours unless someone have anything to say.

danielkcz commented 8 years ago

Yea, I'll save you waiting, I think it looks good, it just feels weird to have any code in here. Eric wasn't against the idea of leaving only specification here, but if you want to maintain it here as well, it's your choice.

koresar commented 8 years ago

@FredyC What's your proposal? Where the example implementation better reside?

danielkcz commented 8 years ago

I don't think there needs to be any example implementation really, especially after that most performant compose is made, I don't see reason why would anyone wrote things differently and yet conform to spec.

koresar commented 8 years ago

True. But let's keep this example implementation here for a while. I go to it myself from time to time. Mostly when I do presentation(s) about stamps on various meetups/conferences and people are asking questions. So, I just show them that file. I need it. :)