Decaffeinate #236
Decaffeinate #236
Conversation
decaffeinate . --use-js-modules --loose
decaffeinate . --modernize-js
Get-ChildItem *.coffee -Recurse | foreach { Remove-Item -Path $_.FullName }
aminya
force-pushed
the
decaffeinate
branch
4 times, most recently
from
5a17396 to
b8ee62d
Compare
|
@aminya thanks for the contribution, however to make the review process more effective and minimize risks I would ask you to follow the advice shared on this PR. In general decaffeination would best happen on stages, to let the users harshly test it out and make sure there are no regressions, another approach if you want to keep things organized is to create an issue tracking your progress for a certain package or code base. an example for a tracking issue, first let everyone know that this issue is to track
I hope that helps |
aminya
force-pushed
the
decaffeinate
branch
2 times, most recently
from
be21b5d to
fab8f99
Compare
Now that we have babel we can use optional chaining
Make formatting like the original code
Make the formatting like the original code
|
@lkashef This is now changed to be only decaffeination. I cannot remove babel since optional chaining relies on that. Now, I used babel-preset-atomic to simplify everything.
Note only related to status-bar PR:
With that being said, I want to state that there should be some level of trust between us. If for some reason there was a regression, we could fix it quickly. If we want to stay conservative, these pull requests will not go anywhere. |
|
@aminya Thanks for sharing these insights and alternative approaches, I like the first approach you suggested also updating the description about the content, these are certainly helpful to proceed with this PR
Regarding the above two lines and other PRs in general, there is indeed a level of trust, otherwise we won’t be investing the time to review the PR; we hold the responsibility of 3M+ active users that depend on us to do our job without breaking their workflows, so I hope you can understand why we might want to be somewhat conservative in our processes here and it’s a process we adapted and improved over time that brought Atom to what it is today. One perspective I would like to share with you, we prioritize bug fixes and stabilizing Atom, as it’s a core that the community can build on top of it, so decaffeination, performance enhancements and features that could be implemented in it’s own community package, such PRs could go behind in line before many bug fixes PRs, things like electron upgrades, and open issues. We are open to improve things, use modern code, approaches, improve performances, etc and we try to find the balance between allowing passionate contributors to get some what non-high priority PR reviewed and merged to show some appreciating and respect for the time they invested, but the priority will always go to make things stable so both Atom team and the community can build on top of something stable and trusted |
Ready
Description of the Change
Decaffeination: rewrites the codebase in JavaScript
Rollup: bundle the package using rollupMoved to separate PRHow to review?
Tests are untouched to show full functionality and backward compatibility. ✔️
If you wanna check commit history:
Remove guard: using if checks instead of optional chaining in CoffeeScript=-> now we use optional chaining using babelif (variable)instead ofif (variable != null)whenever it is safeAlternate Designs
Benefits
- Faster loading time because of using JavaScript and Rollup.Possible Drawbacks
Optional chaining isn't as compact as CoffeeScript in JavaScript. I rewrote them using=> Now use optional chaining using babelifchecks. By switching to TypeScript/or a new Babel version we can write them more compact.Applicable Issues
Updates the package to make it ready for atom/atom#18282
Closes the old PR: #215