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 the debug option and verbose (foreground) #1949

Merged
merged 8 commits into from
Oct 8, 2021

Conversation

folker-kuhn
Copy link
Contributor

@folker-kuhn folker-kuhn commented Oct 5, 2021

Fixes #1954

I am proposing this PR, so we can debug and do foreground run (or start?) easily. I took "suspend" and "verbose" names from some other projects, so I think the user is familiar with it.

I am proposing this PR, so we can debug and do foreground run (or start?) easily. I took "suspend" and "verbose" names from some other projects, so I think the user is familiar with it.
@Thihup
Copy link
Collaborator

Thihup commented Oct 5, 2021

Hi @folker-kuhn.

As this is your first PR can you please acknowledge that the code will be owned by Manorrock.com. I will only ask this one time, as that is all that is needed for contributing code back to us.

About running in foreground, the run.sh does exactly that, so that it wouldn't be needed.
Also, could you implement the suspend part it in the run.sh script too?

@folker-kuhn
Copy link
Contributor Author

folker-kuhn commented Oct 6, 2021

I acknowledge the code in this PR and all further PRs I do to this project will be owned by Manorrock.com. Thanks!

Okay, that is what run.sh is for. I have to say that having both start and run is somewhat confusing. If you must have separate scripts for this, maybe start-bg and start-fg would be easier to grasp.

I'll look at run.sh

@mnriem
Copy link
Contributor

mnriem commented Oct 6, 2021

@folker-kuhn Thanks for the acknowledgement. I'll let @Thihup work with you on the PR as he already engaged. We can now start merging any of your contributions. Welcome and thank you!

Adding --run option to start in background with a wait
Make sure all options in start.sh also apply when using run.sh
Touch the pid before the command, since there is some loop looking for it in the cmd, and the pid is otherwise created after it. We get a race here.
Copy link
Collaborator

@arjantijms arjantijms left a comment

Choose a reason for hiding this comment

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

LGTM

folker-kuhn and others added 2 commits October 7, 2021 15:23
Co-authored-by: Thiago Henrique Hüpner <thihup@gmail.com>
@folker-kuhn
Copy link
Contributor Author

@Thihup what happened with the last commit and test run?

@folker-kuhn
Copy link
Contributor Author

Error: Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.3.0:jar (attach-javadocs) on project piranha-pico-core: MavenReportException: Error while generating Javadoc:
Error: Exit code: 1 - Loading source file /home/runner/work/piranha/piranha/pico/core/src/main/java/module-info.java...
Error: Constructing Javadoc information...
Error: error: No public or protected classes found to document.
Error: 1 error

I don't think this is related to my commit?

@Thihup
Copy link
Collaborator

Thihup commented Oct 7, 2021

@folker-kuhn It looks like @mnriem added a new subproject. However, it does not have any classes yet. So the Javadoc is complaining about it. It is not related to your changes. Sorry for the confusion.

@arjantijms arjantijms merged commit 2342d13 into piranhacloud:current Oct 8, 2021
@mnriem
Copy link
Contributor

mnriem commented Oct 10, 2021

@folker-kuhn Thank you for your first contribution! It is MUCH appreciated and sorry for the confusion with the build ;)

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.

Add the debug option and verbose (foreground)
4 participants