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

Fixes for #3954 #3974

Merged
merged 7 commits into from
May 22, 2020
Merged

Fixes for #3954 #3974

merged 7 commits into from
May 22, 2020

Conversation

DarkWeird
Copy link
Contributor

Fixes for #3954

  • misc.xml to git ignore - saves Project JDK
  • jarRepositories to git ignore - regenerates/generates by idea
  • Choose shortenClasspath used by java8 and java9+

@keturn
Copy link
Member

keturn commented May 21, 2020

Ah, I wondered if the repositories file was managed by gradle. If it is, and it updates as it syncs with gradle about which repositories the project uses, then yes, it is appropriate to ignore.

as for shortenClassPath: We can certainly set this back to "default" if that is necessary information. I thought it looked like a good setting to reduce the clutter in the run's output window. Is that information you need to refer to on a regular basis?

Change engine test runconfiguration for run all tests
@DarkWeird
Copy link
Contributor Author

as for shortenClassPath: We can certainly set this back to "default" if that is necessary information. I thought it looked like a good setting to reduce the clutter in the run's output window. Is that information you need to refer to on a regular basis?
I'm don't understand you question.

@DarkWeird DarkWeird added the Type: Bug Issues reporting and PRs fixing problems label May 21, 2020
@keturn
Copy link
Member

keturn commented May 22, 2020

We had a lengthy chat about this in #stabilization, I'll try to summarize here for the record:

  1. We agreed it is okay for this IDEA configuration to use JDK 11. (related: update build.gradle "compiling with a JDK newer than" warning for jdk11 #3975)
  2. We'd like to avoid removing misc.xml entirely, because we'd lose the bit about annotation entry points. (I don't think https://github.com/JetBrains/gradle-idea-ext-plugin offers any clean alternative for setting those.)
  3. the bellsoft liberica JDK isn't necessary for engine and the PC facade, they do not involve javafx. We'll change project-jdk-name from liberica-11 to 11, which is the default thing IDEA will suggest if it finds a system installation of openjdk-11.

I think we still have an outstanding concern that other plugins the developer may have installed will write things to other component sections of misc.xml?

If the things they write are suitable values for the project, I'm okay with adding those sections as they come. But if they frequently change, or the plugin is mistakenly storing stuff in there that's not really project related (e.g. should be workspace or ~/.config), or if someone else's IDEA removes any sections for plugins not currently installed, then we really can't keep misc.xml directly in version control.

@keturn
Copy link
Member

keturn commented May 22, 2020

As for the "All in NameGenerator" run config:

@Cervator, I know you said it's good to have an example here of how to do a run config for all the tests in a module, but it does look odd if you don't have NameGenerator installed 😄. Should we change this to BuilderSimpleGameplay instead?

or is BSG special in some way that means it would not be a good example? (because it is included in the repo)

@Cervator
Copy link
Member

@keturn I'm fine removing the NameGenerator test run config 👍

BSG doesn't have any test, and very little actual code to test. It isn't really special, it is just a minimal example of running the game. I guess we could try to validate that it can load its dirt and grass blocks? Heh :-)

As for misc.xml there's precedent for the weirdness of first checking in a file, then separately .gitignoring it - at least in the past I think it did work, even if it was kinda odd.

@DarkWeird
Copy link
Contributor Author

DarkWeird commented May 22, 2020

We had a lengthy chat about this in #stabilization, I'll try to summarize here for the record:

1. We agreed it is okay for this IDEA configuration to use JDK 11. (related: #3975)

2. We'd like to avoid removing `misc.xml` entirely, because we'd lose the bit about annotation entry points. (I don't think https://github.com/JetBrains/gradle-idea-ext-plugin offers any clean alternative for setting those.)

3. the bellsoft liberica JDK isn't necessary for engine and the PC facade, they do not involve javafx. We'll change project-jdk-name from `liberica-11` to `11`, which is the default thing IDEA will suggest if it finds a system installation of openjdk-11.

I think we still have an outstanding concern that other plugins the developer may have installed will write things to other component sections of misc.xml?

If the things they write are suitable values for the project, I'm okay with adding those sections as they come. But if they frequently change, or the plugin is mistakenly storing stuff in there that's not really project related (e.g. should be workspace or ~/.config), or if someone else's IDEA removes any sections for plugins not currently installed, then we really can't keep misc.xml directly in version control.

Arch Linux: IDEA's download jdk feature not work.
all JDKs in pattern "OpenJDK (Version)"
IDEA installed via package manager

@DarkWeird
Copy link
Contributor Author

ehhhh.
Jetbrains have build instruction with "configure JDK" :(
https://github.com/JetBrains/intellij-community

chore(.idea): Change README for info about Java and JDK within IDEA
.idea/misc.xml Outdated Show resolved Hide resolved
@DarkWeird DarkWeird merged commit c2d6840 into develop May 22, 2020
@DarkWeird DarkWeird deleted the topic/idea-fixes branch May 22, 2020 20:53
@Cervator Cervator added this to the v3.2.1 milestone May 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues reporting and PRs fixing problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants