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

Add explicit constructor #148

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

adamoutler
Copy link

@adamoutler adamoutler commented Mar 23, 2022

Fixes a message with this class when modularized
se/vidstige/jadb/Subprocess.java:[23,8] class se.vidstige.jadb.Subprocess in exported package se.vidstige.jadb declares no explicit constructors, thereby exposing a default constructor to clients of module my.module.

Fixes a message with this class when modularized
se/vidstige/jadb/Subprocess.java:[23,8] class se.vidstige.jadb.Subprocess in exported package se.vidstige.jadb declares no explicit constructors, thereby exposing a default constructor to clients of module my.module.
@vidstige
Copy link
Owner

Thanks for your contribution. The warning seems weird, where did you see this warning?

@adamoutler
Copy link
Author

Java 11. I converted, modularized, and imported to my codebase before you added the changes for automodule. I'm contributing back compatible changes.

You will eventually want to upgrade to Java 11, even if only in a docker container on Actions on GitHub. Minor changes are needed for compatibility and like this, but the major item is the module-info.java. Aside from the module-info, it's just a bunch of lint and syntactic faux pas that have no bearing on operation, but make it more compatible with lint checks.

@vidstige
Copy link
Owner

I see. Thanks for the initiative and your contribution. I'm currently not working at all on this so any moves to new Java versions is entirely dependent on contributors like yourself. PRs welcome!

For the future - Avoid comments that are not very helpful like // empty to keep the code clean and easy to read.

Copy link
Owner

@vidstige vidstige left a comment

Choose a reason for hiding this comment

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

I think the public Subprocess(){} constructor needs to be inside the class Subprocess? The build is currently failing.

Please amend the commit. 🙏

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.

2 participants