Skip to content

Updated NUI to use gestalt 7#4

Merged
skaldarnar merged 6 commits into
MovingBlocks:masterfrom
BenjaminAmos:nui-extraction-gestalt7
Aug 9, 2020
Merged

Updated NUI to use gestalt 7#4
skaldarnar merged 6 commits into
MovingBlocks:masterfrom
BenjaminAmos:nui-extraction-gestalt7

Conversation

@BenjaminAmos
Copy link
Copy Markdown
Contributor

@BenjaminAmos BenjaminAmos commented Jul 20, 2020

Most of the changes in this pull request involve updating NUI to use the new gestalt namespace, as well as changing the usages of a couple of re-named methods. To avoid conflicts with the 1.0.0 version of NUI, which still uses an older incompatible version of gestalt, the version number is now 2.0.0. There should be no functional UI changes in this pull request, apart from using a newer version of gestalt.

This is needed for MovingBlocks/DestinationSol#536.

Copy link
Copy Markdown
Member

@skaldarnar skaldarnar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes itself look good to me 👍

The reason why I requested changes is because I want to clarify what that means for releases of TeraNUI and how it is used in both Terasology and DestSol.

My understanding is that Terasology, as long as it is with gestalt v5, will use TeraNUI v1. DestSol, on the other hand, is backed by gestalt v7 and will use TeraNUI v2.

Both these TeraNUI versions currently have feature partiy, they basically are the same code. Any changes should probably be present in both branches for v1 and v2. I've never worked with a library that needs to support multiple versions, that's why I'm asking for some documented best practices.

  • Where do we do the main development? I guess a main branch with the latest code (on gestalt v7)...
  • How do we maintain old versions? I assume there'll be a branch for TeraNUI v1 based on gestalt v5...
  • How to back-port changes? That's a question I'm always asking myself when submitting something to gestalt. I'd like to have a least a rough guideline how to backport a change to TeraNUI v1 so that every contributor with write permissions on this repo can do this.
  • How do releases work? Do we need some special Jenkins setup here? How is that solved for gestalt itself?

Pinging @Cervator @jdrueckert @immortius for some feedback on this (and please let me know if "blocking" this PR is too harsh 🙈 )

@Cervator
Copy link
Copy Markdown
Member

Good questions there, and I've wondered some myself. The initial release reworking I did on TeraNUI since good candidate for experimentation just gets the releases off master based on a simple check on the version string (same as and in fact copy pasted from Gestalt)

My next goal along with this PR was going to be how to handle releases in parallel and would likely involve checking a branch pattern beyond master in Jenkins, so a branch like release_v1.2.3 (or maybe rc_v1.2.3 ? The moment it finally releases and gets tagged the branch could be deleted) would also qualify for publishing snapshots and ultimately a separate release, limited to a version matching the branch name (to avoid oopsies like making a release branch without changing the version in code)

That doesn't solve maintaining the two releases though. That reminds me a little of how the https://github.com/MovingBlocks/CrashReporter actually has little mini-wrappers for Terasology and DestSol yet they live in the same branch, there's only one build, but it publishes a generic binary + the game specific wrappers, and each game just targets that to get its unique properties and such. You might be able to resolve the different Gestalt dependencies that way but not readily the differing package names and actual logic changes, so that may not help :-(

After I pushed forward a bit here I aimed to apply the lessons learned to the Gestalt repo, as yeah, it got a little messy with the branches and such over time. While we could probably come up with all kinds of clever ways to handle parallel streams I'm not sure either project is likely to change enough to throw more logistics at it, maybe it would be enough to just manually backport / pull forward changes made in one stream to the other. https://xkcd.com/974/

Will try to first just support parallel releases at all, then poke at Gestalt, and see how things are looking by then on how to handle ongoing development. Surely we'll need it elsewhere in the future, so it'll be good to have some best practices. More ideas and/or experimentation definitely appreciated 👍

@skaldarnar
Copy link
Copy Markdown
Member

I was rather hoping for a simple markdown document guiding me through some steps instead of full automation 🙄 🙈 I don't think that we should attempt templating the NUI code to generate artifacts for gestalt v5 and v7. A simple guide on how to merge and back-port changes would be enough.

Also, I'm not sure we need automated parallel releases (not exactly sure what you mean by "parallel releases" tbh). I'd think we'll have release branches from which we do releases for teranui-1.x and teranui-2.x respectively. This my require to commands or actions by the maintainer, all fine as long as it is documented.

I'd prefer a git-tag based approach, but I'm not sure whether we are there yet 🤓

@Cervator
Copy link
Copy Markdown
Member

Oh certainly agreed on a proper write-up in some doc. I don't personally have anything firm I can write up yet, but we'll get there at some point soon I'm sure. Right now mostly just thinking out loud.

Yeah with parallel releases feel free to interpret as some sort of release branch setup, didn't have anything special or elaborate in mind. Point being we'll occasionally work on either of the two lines. Hopefully we can do something simple like cherry picking commits to backport.

@Cervator
Copy link
Copy Markdown
Member

Cervator commented Aug 2, 2020

Small side-comment: I've done a from-local snapshot release of this branch to Artifactory so 2.0.0-SNAPSHOT is available for further DestSol tinkering. Will get back to a more formal release process when able and sort out how to merge this by then :-)

@skaldarnar
Copy link
Copy Markdown
Member

skaldarnar commented Aug 9, 2020

@BenjaminAmos any idea what might be off with this?

> Task :nui:test FAILED
:nui:test (Thread[Execution worker for ':',5,main]) completed. Took 3.735 secs.

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':nui:test'.

> Could not resolve all files for configuration ':nui:testRuntimeClasspath'.
   > Could not find com.android.support:support-annotations:28.0.0.
     Searched in the following locations:
       - file:/home/jenkins/.m2/repository/com/android/support/support-annotations/28.0.0/support-annotations-28.0.0.pom
       - https://jcenter.bintray.com/com/android/support/support-annotations/28.0.0/support-annotations-28.0.0.pom
       - https://repo.maven.apache.org/maven2/com/android/support/support-annotations/28.0.0/support-annotations-28.0.0.pom
       - http://artifactory.terasology.org/artifactory/virtual-repo-live/com/android/support/support-annotations/28.0.0/support-annotations-28.0.0.pom
       - http://maven.snplow.com/releases/com/android/support/support-annotations/28.0.0/support-annotations-28.0.0.pom

     Required by:
         project :nui > org.terasology.gestalt:gestalt-module:7.0.3
         project :nui > org.terasology.gestalt:gestalt-asset-core:7.0.3
         project :nui > org.terasology.gestalt:gestalt-module:7.0.3 > org.terasology.gestalt:gestalt-util:7.0.3

@pollend added some test dependencies in #8, maybe that makes problems now?

Gestalt 7 uses annotations from Android's support-annotations library internally, which can only be found in Google's Maven repository.
@BenjaminAmos
Copy link
Copy Markdown
Contributor Author

I had not included any tests previously, so I hadn't tried running them before. Gestalt 7 makes use of some annotations from the Android support libraries (https://developer.android.com/studio/write/annotations) in order to enforce certain warnings with the linter (like the @RequiresApi annotation here). Unfortunately those annotation libraries are only found in the google maven repository. The libraries compile fine without them but when it comes to the tests then they are required on the classpath. The simplest fix for this is to include the google repository in the build script and I've just pushed a commit to do that. The tests don't produce that error for me after adding the repository in.

@skaldarnar skaldarnar merged commit 926d327 into MovingBlocks:master Aug 9, 2020
Cervator pushed a commit that referenced this pull request Aug 24, 2020
* Converted NUI to use gestalt 7.0.3
* Updated library version to 2.0.0 due to gestalt upgrade (now using gestalt 7)
* Fixed compiler errors due to changed gestalt namespace
* Added google maven repository.
Cervator pushed a commit that referenced this pull request Aug 26, 2020
* Converted NUI to use gestalt 7.0.3
* Updated library version to 2.0.0 due to gestalt upgrade (now using gestalt 7)
* Fixed compiler errors due to changed gestalt namespace
* Added google maven repository.
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.

3 participants