Skip to content

Conversation

@jjzazuet
Copy link
Contributor

  • Baselining at Java 8.
  • Documentation updates.
  • Initial Bintray OSS release (awaiting Bintray request approval).

I think we'll need to create an issue to split the core README.md file into smaller feature specific files. This should also allow the documentation to be compatible with gitbooks.

I'll add more details to that issue later. Thanks.

- Baselining at Java 8.
- Documentation updates.
- Initial Bintray OSS release (awaiting Bintray request approval).

I think we'll need to create an issue to split the core `README.md` file into smaller feature specific files. This should also allow the documentation to be compatible with gitbooks.

I'll add more details to that issue later. Thanks.
@jjzazuet jjzazuet requested a review from simonwep September 27, 2020 17:16
@jjzazuet
Copy link
Contributor Author

Ok, TravisCI builds are good now. Thanks.

https://travis-ci.org/github/Simonwep/java-express/builds

@jjzazuet
Copy link
Contributor Author

Also, Bintray has approved the artifact publication. Nice.

https://bintray.com/search?query=java-express

@simonwep
Copy link
Owner

Wow, good job with all that! It's great seeing this project being alive again :D I'll give it a quick review tomorrow!

Copy link
Owner

@simonwep simonwep left a comment

Choose a reason for hiding this comment

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

Do we need to keep binary files (gradle-wrapper.jar) and script files such as gradlew and gradlew.bat in the repository? They seem to be rather generic (I'm not really familiar with Gradle, just a general though) or are they crucial and fix / unique for this repository?

@jjzazuet
Copy link
Contributor Author

@simonwep they are. At least until we can get TravisCI to upgrade its Gradle version up to at least 6.6.1. By default, right now it's running under Gradle 5.X. If we can do that, then we can remove the wrapper files from the source tree. Thanks.

@simonwep
Copy link
Owner

Oh, I almost forgot about that - could we switch to GitHub action? This way we'd have better control over CI and could fix the Gradle issue!

@jjzazuet
Copy link
Contributor Author

I think so. I'll make the changes on the PR. I'll ping back if I need you to grant me admin access to the repo. Thanks!

@jjzazuet
Copy link
Contributor Author

jjzazuet commented Sep 28, 2020

@simonwep ok I'm putting together a basic github actions template, but I need to configure credentials for artifact publishing. When you have a moment, please grant admin access to my user on this repository. I'll then check in the workflow YAML file for building and uploading snapshots (which should also allow for removing the Gradle wrapper files), and remove the TravisCI monitoring hooks.

Cheers.

- Minor code cleanup.
@jjzazuet
Copy link
Contributor Author

Also pushed a couple of commits to replace java.util.logging with the SLF4J api (it's a modern minimal API contract, consumers choose their own logging back end). Thanks!

@simonwep
Copy link
Owner

Looks really good! You should already have access to Actions (just checked with another gh-account, you might have to create a workflow in the action tab?) - If I'm not mistaken admin rights are only a thing in organizations, collaborators are basically admins (except that they can't configure the repo).

@jjzazuet
Copy link
Contributor Author

@simonwep yeah, all I'm looking for is to be able to tweak the settings tab in this repo to remove the TravisCI web hooks. You can do that as well. I'll check in the Github Actions yaml files later during the week. Thanks!

@simonwep
Copy link
Owner

Gotchya, I've removed the webhook :)

@jjzazuet jjzazuet merged commit 03571e9 into master Oct 3, 2020
@jjzazuet jjzazuet deleted the build-system branch October 3, 2020 04:01
@simonwep
Copy link
Owner

simonwep commented Oct 3, 2020

Good job! What would come next?

@jjzazuet
Copy link
Contributor Author

jjzazuet commented Oct 4, 2020

A few ideas:

  1. Issue Async support #11 is possibly referring to implementing non-blocking requests, where a blocking requests refers to something like reading from a DB connection, or executing some other long running code that will block a thread processing an HTTP request. The idea is to create a second task processing pool where such tasks can be moved, and then sending the response over once the blocking code completes. This frees up the threads that process HTTP requests to continue accepting tasks, while the heavy work occurs elsewhere.
  2. I think we could move the basic set of middleware implementations to its own package (something like java-express-middleware-base), so that people can choose to use the default set of middle-ware functions, or provide their own. For example, I need a middle-ware request handler which can serve static content from a jar's classpath location (i.e. a public folder inside a jar file. I'm writing a prototype handler, so I may not have much need for the FileProvider middle-ware that comes out of the box.
  3. I was thinking of applying the googleJavaFormat plugin on the first PR, but it would make the file count too large and hide the important changes. I think it's useful to validate and enforce code formatting for new PRs. This plugin only needs a single line to activate and provides automatic code formatting:
configure<io.vacco.common.CbPluginProfileExtension> {
  addJ8Spec(); addPmd(); addSpotBugs()
  addGoogleJavaFormat() <------------------------------------------------------- HERE
  setPublishingUrlTransform { repo -> "${repo.url}/${project.name}" }
  sharedLibrary()
}

Got some more stuff in mind, but this should be enough to get going. I'll either open new issues, or submit the PRs directly.

Thanks!

@simonwep
Copy link
Owner

simonwep commented Oct 8, 2020

  1. Okay, what would the solution to this issue look like? Is this a rather envieronment-specific issue or are Futures in Java a thing and we should add them to the api / make them generally usable with java-express?
  2. Oh yeah, that's good! We should definitely separate the core-code from the extensions!
  3. Just saw your PR, looks great! commented on it :)

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