-
Notifications
You must be signed in to change notification settings - Fork 463
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
Remove refmt.exe / Reason syntax support #5683
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's also some stuff that is used for detecting dependencies between files, i.e. in the builder or somewhere. Not super important, but it becomes essentially dead code.
Let me try to find it.
@@ -16,6 +16,5 @@ | |||
"warnings": { | |||
"error" : "+101" | |||
}, | |||
"namespace": true, | |||
"refmt": 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens to this config option? I think it should just be ignored for now -- no need to introduce a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it was already ignored and could safely be removed? I don't have this setting in any of the bsconfig.json files in our projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I did not mean to imply that it is not ignored. It was just a thought, to check that it is indeed the case.
Looking in let suffix_re = ".re"
let suffix_rei = ".rei" should get rid of a bunch of dead code. |
53797fa
to
67e34eb
Compare
d7efdd4
to
0018aa3
Compare
I guess this requires a good entry in the changelog, including how to use an older version of the compiler to convert legacy Reason projects. |
* Remove bsrefmt * Remove refmt.exe * Remove "refmt": 3 * Remove ast_reason_pp module and re/rei file extensions * Remove leftover js_refmt_compiler * Remove refmt_main3.ml * Fix moveArtifacts script * Clean up literals.ml * Update CHANGELOG
Why? If people are willing to maintain a converter between reason and rescript why force it? |
@danielo515 It is more than two years where we announced our plans to put our focus on a homogenous ecosystem with one curated, well supported syntax (which is ReScript). All our infrastructure (including editor-support) is relying on We decided that we don't have the capacities to continue this maintenance effort, and it's limiting us greatly in improving our build system toolchain and allowing easier support for ARM based binaries (which will definitely be way more important to our core audience than supporting .re files). So it's mostly an economic question of what we want to spend our time on, how we can make our development effort more streamlined, and most importantly, how to get the most bang for our bucks. This is one huge stepping stone to get there. Since this is a major version release, we don't expect anyone to upgrade right away, so there's enough time to upgrade all the libraries that are still running on |
Thanks for the detailed answer.
I was not meaning that you should maintain or offer support for the refmt,
but, if it can be a completely isolated an external program, and it does
not limit the development of rescript, then why not let it exist, just
forget that it exist and continue evolving as you wish. However, if you
think it limits the development of rescript in any way, then ok,
I understand that you remove it.
Greetings
…On Fri, Oct 7, 2022 at 4:03 PM Patrick Ecker ***@***.***> wrote:
@danielo515 <https://github.com/danielo515> It is more than two years
where we announced our plans to put our focus on a homogenous ecosystem
with one curated, well supported syntax (which is ReScript). All our
infrastructure (including editor-support) is relying on .res files and we
provide all the tools to migrate from .re to .res OOTB as well.
We decided that we don't have the capacities to continue this maintenance
effort, and it's limiting us greatly in improving our build system
toolchain and allowing easier support for ARM based binaries (which will
definitely be way more important to our core audience than supporting .re
files). refmt also made things way bulkier... e.g. the playground JS
bundle doubles in size due to the vendored Reason source code (the RE
syntax is heavier than the compiler + ReScript syntax together due to the
way its vendored). The refmt.exe only works on MacOS M1 because of
Rosetta, and I believe it doesn't even work on Linux ARM.
So it's mostly an economic question of what we want to spend our time on,
how we can make our development effort more streamlined, and most
importantly, how to get the most bang for our bucks. This is one huge
stepping stone to get there.
Since this is a major version release, we don't expect anyone to upgrade
right away, so there's enough time to upgrade all the libraries that are
still running on .re syntax anyways.
—
Reply to this email directly, view it on GitHub
<#5683 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AARKJWOLUTKZCR5A7GUGWM3WCAUUVANCNFSM6AAAAAAQSWXBQM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
---
https://danielorodriguez.com
|
* Remove bsrefmt * Remove refmt.exe * Remove "refmt": 3 * Remove ast_reason_pp module and re/rei file extensions * Remove leftover js_refmt_compiler * Remove refmt_main3.ml * Fix moveArtifacts script * Clean up literals.ml * Update CHANGELOG
We'll finally be removing support for legacy Reason syntax in ReScript 11.
Existing projects using Reason syntax can be converted to ReScript syntax using the
rescript convert
command in ReScript 9.