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

Move out Playground from the compiler #6455

Closed
wants to merge 1 commit into from

Conversation

j8r
Copy link
Contributor

@j8r j8r commented Jul 27, 2018

The playground will live in a separate repository, for now at https://github.com/j8r/crystal-playground
There are still minor issues to fix.

There are upsides/downsides of moving out the playground.

Pros:

  • Remove web related JS/CSS/HTML code of the compiler
  • More time to focus on the Crystal language itself by removing JS/CSS/HTML issues and PRs
  • Reduce the compile time and binary size of the compiler
  • Remove the HTTP::Server dependency from the compiler
  • All without_openssl could also be removed

Cons:

  • complicates the release cycle
    ▶️ Also remove playground from the tarball release.
    The release tarball will be also marginally smaller.

  • I like crystal play and I want to keep using it
    ▶️ You will be able to do either:
    brew install crystal-playground
    apt install crystal-playground
    or clone https://github.com/crystal-lang/crystal-playground and crystal build it

Some stats:

make release=true time afer a make clean on an Alpine Edge Docker container:

no playground:

real    9m 19.89s
user    9m 17.93s
sys     0m 4.60s

crystal binary size: 24.416240M

with playground:

real    9m 39.98s
user    9m 35.07s
sys     0m 5.08s

crystal binary size: 25.117896M

Relative discussion: #6083

The tool will live in a separate repository
@straight-shoota
Copy link
Member

I think this PR is too premature. There is no decision made neither in #6083 (which is originally an unrelated issue) nor anywhere else. And making a decision requires more discussion about how the compiler, compiler tools and standard library should be developed and integrated. This also touches #5215 and #4613. Until there is a plan for all of this, I don't think we need any PR.

@j8r
Copy link
Contributor Author

j8r commented Jul 27, 2018

@straight-shoota sorry I don't quite understand what you mean. The change is only about the Playground, nothing else.
People thoughts on this are quite clear, 19 👍 to move out vs 3👎 to do nothing.

This doesn't stop to have the same discussion again here of course, but I'm afraid having general discussions won't lead to any concrete actions.

Please let's talk on what the PR is about here – move out the Playground, and its dozens of related issues. One thing at a time...

@Sija
Copy link
Contributor

Sija commented Jul 27, 2018

@j8r I agree with @straight-shoota. Whatever the actions needed to be taken, IMHO they should be stewarded by someone from @crystal-lang core team, not outside contributors and for sure the repo shouldn't be outside of @crystal-lang org.

@j8r
Copy link
Contributor Author

j8r commented Jul 27, 2018

BTW some core members has already expressed opinions:
@asterite, @bcardiff and RX14

The creation of https://github.com/crystal/playground has been asked

We know what most people think on this, the decision is up to @crystal-lang members now.

@Sija
Copy link
Contributor

Sija commented Jul 27, 2018

BTW some core members has already expressed opinions:
@asterite, @bcardiff and RX14

Opinions are not actions.

The creation of https://github.com/crystal/playground has been asked

Yet not answered (yet), hence 404 on your link.

We know what most people think on this, the decision is up to @crystal-lang members now.

Amen.

@RX14
Copy link
Contributor

RX14 commented Jul 30, 2018

The playground is intrinsically linked to the compiler internals. Putting it inside a different repository will just make refactoring the compiler harder. This is a bad idea.

@asterite
Copy link
Member

The playground is what makes all those if flag openssl needed. The playground can catch up to compiler changes. It really doesn't belong in the compiler.

@bcardiff
Copy link
Member

bcardiff commented Jul 30, 2018

I really appreciate that the playground is extracted in a new repo while keeping the history.

But, this PR can't be accepted until:

  1. the distribution scripts are updated to deliver the same experience ($ crystal play)
  2. a ci in playground is setup to use nightly releases of the compiler to avoid issues on release
    or define brew formulas / linux packages to distribute the playground, which would lead probably to have a compiler, playground and whole bundle packages.

Also, as stated here:

I would like to see this done only and as long as the compiler is extended to let the program be instrumented in a better way as it is today. Otherwise changes to the language or the the ToSVisitor are too sensible to break the playground.

Another cons of splitting is that the bin wrapper of the compiler will need to be duplicated for the playground probably, and actually the playground will have a fully featured compiler, but is not the compiler...

I don't think we are going to invest in the required tasks mention earlier so unless those want to be tackled by someone, this leave things stalled for this PR for the moment.

I acknowledge the will to split it from the compiler, but I don't think is going to happen soon.

@j8r
Copy link
Contributor Author

j8r commented Dec 20, 2018

It could happen in the far future, not anytime soon – closing.

@j8r j8r closed this Dec 20, 2018
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

6 participants