Removed VisualVM and JDK, changed mirror for Eclipse

ioi-2023 / contestant-vm

Contestant virtual machine for online IOI (since 2020)

1 stars 2 forks source link

Removed VisualVM and JDK, changed mirror for Eclipse #54

Closed horcsinbalint closed 1 year ago

horcsinbalint commented 1 year ago

Should fix #48.

radl97 commented 1 year ago

To the changes to eclipse: downloading eclipse will fail one year later (very probably), because as part of their rollout process, different Eclipse installations will be available for download. This makes the process fragile.

I think that the removal of JDK, while understandable, is not crucial to warrant for a more fragile script.

However, that is only my 2 cents, and I did not get to follow up on the discussion about this.

horcsinbalint commented 1 year ago

It's probably a faster mirror

I think this loses context in a comment. However, what is the background on this? (when I downloaded, twice I got the liteserver nl link). You got this one instead? Or there was something less trivial? (I think it's not worth changing mirrors in a set of 4 commits in the same PR, without enough context)

I have tried two different mirrors, and this one was much faster, than the old one.

horcsinbalint commented 1 year ago

To the changes to eclipse: downloading eclipse will fail one year later (very probably), because as part of their rollout process, different Eclipse installations will be available for download. This makes the process fragile.

I think that the removal of JDK, while understandable, is not crucial to warrant for a more fragile script.

However, that is only my 2 cents, and I did not get to follow up on the discussion about this.

Since the 2020-09 release, JRE for eclipse has been delivered in a similar way (using JustJ) if someone downloaded the .tar.gz. I think you had to bring your own JRE prior to that. Unfortunately, they are also delivering JDK with their JRE, that's why I tried to bring a different JRE instead. If eclipse is installed using the UI, you can select your JRE instead.

I will check the older releases but I think by some small improvements, the stability of this script can be drastically improved.

pobrn commented 1 year ago

If possible, could you please squash the commits that fix previous commits?

pobrn commented 1 year ago

Also, if a commit fixes an issue, then please put

Fixes #9999 # of course use the real ID instead of 9999

in the commit message so that when the commit is merged, the issue is automatically closed.

radl97 commented 1 year ago

@pobrn :

Regarding squashing: it's a nice-to-have, but I would not require it. There is an alternative to squash everything:

I think manual squashing is too cumbersome to be something to require, but I agree that it's a nice-to-have.
I prefer linking the issue to the PR instead of commits, as you have suggested.

@horcsinbalint : I do not remember our previous discussions and decisions. Is this PR out of date in every aspect? Or is there still relevant stuff?

pobrn commented 1 year ago

Regarding squashing: it's a nice-to-have, but I would not require it. There is an alternative to squash everything:

Well, my preference is strongly "Rebase and merge". Squashing multiple commits, especially when they are not really related (like here) is not something I would do.

I think manual squashing is too cumbersome

Maybe I am just too used to it, but an interactive rebase is very easy in my opinion.

If upstreaming these changes into the parent repository is planned, then I think it makes sense to keep the commit history somewhat sane.

I prefer linking the issue to the PR instead of commits, as you have suggested.

That is fine as well, but then PRs should be reasonably limited in scope.

Anyways, I feel like this is already too much bikeshedding. 🙃

radl97 commented 1 year ago

We have discussed a simpler solution. I have changed the mirror and removed VisualVM.