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

Stampit v4 #326

Merged
merged 38 commits into from
Nov 9, 2017
Merged

Stampit v4 #326

merged 38 commits into from
Nov 9, 2017

Conversation

koresar
Copy link
Member

@koresar koresar commented Oct 21, 2017

Closes #323 #317 #304 #68

Changes:

  • Removed the previously deprecated refs.
  • Removed stampit/* utility functions. Advice to use @stamp/* modules instead.
  • composers are not marked as experimental any more.
  • Stampit becomes compatible with all @stamp/* stamps.
  • The .min.js bundle size reduced by ~50%, the gzipped size reduced by ~40%.
  • Tests can now be manually run in a browser (by opening ./test/index.html)
  • Stampit source code is ES5 now instead of ES6.
  • Fixed perf bottleneck in node.js 8 (new V8 engine).

In case of silence I'll merge this PR in a week.

@danielkcz
Copy link
Contributor

Phew, sorry @koresar, but I don't have a space to review this bulk of changes.

Honestly, I am not even sure why to bother with another stampit release when there is stamp monorepo. Why not to just have a stampit@v4 to reference @stamp/it instead? Why two different code bases?

@koresar
Copy link
Member Author

koresar commented Oct 22, 2017 via email

@danielkcz
Copy link
Contributor

Sorry, but that still does not explain why to keep two almost identical implementations. When I started the idea about monorepo I thought that eventually, stampit module will be just a redirect to this monorepo and after a while it might be even deprecated and removed.

Right now it's kinda unclear where is the difference. Is it possible to use extra modules from the monorepo with stampit? That's an example of a user question, I know they are both up to spec including composers, but in my opinion, it just makes things worse.

I can imagine it wasn't easy to accomplish size reduction you wanted, but it feels kinda OCD :) Do you know someone who actually complained that this lib is couple bytes larger than it could be? I am probably living in a different world where a size of the libs is so much higher and it does not really matter in overall.

Personally, I have kinda lost interest in stamps in overall. Perhaps I've never understood it that well to use it efficiently and without losing grasp over my codebase. In complex scenarios, it was just too messy for my taste. Not even mentioning adoption when working on the team and explaining everyone how it works.

If you feel like you want to go this direction then go ahead and merge and release it. I am not best to judge this anymore. Sorry friend.

@koresar
Copy link
Member Author

koresar commented Nov 9, 2017

Stampit have to jump to next major version only because it is not following the latest spec v1.5.
If we would vote to make "composers" part of the spec year ago stampit would have stayed v3.
Unfortunately, community took a careful decision to do baby steps - implement "composers" as an experimental feature first.
So, stampit v4 is a necessary last baby step towards compatibility with the latest specification v1.5.

@koresar koresar merged commit d4a39be into master Nov 9, 2017
@koresar koresar deleted the simplifying-minifying branch November 9, 2017 11:13
Copy link
Contributor

@AlexxNica AlexxNica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@koresar Why did you remove the license?

@@ -1,21 +0,0 @@
The MIT License (MIT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here

@AlexxNica
Copy link
Contributor

@koresar Little tip: You could have rebased your commits before pushing and merging to avoid polluting the history.

Take a look at:
https://git-scm.com/docs/git-rebase
https://git-scm.com/book/en/v2/Git-Branching-Rebasing

@koresar
Copy link
Member Author

koresar commented Nov 15, 2017 via email

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

3 participants