-
Notifications
You must be signed in to change notification settings - Fork 84
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 TDD non-reg illustrating #213 problem (currently still failing) #216
base: master
Are you sure you want to change the base?
Conversation
@muff1nman you could test your #217 against this? Run If #217 if it fixed it correctly, then by (locally) rebasing / cherry-picking this one on top of that one passes. Shout if this is not clear and ask a question and perhaps we can improve docs for contributors. |
@muff1nman following your #219 I'll rebase this, and if running |
4ac349b
to
72b93c5
Compare
@muff1nman something here isn't quite right yet... if you wouldlike to investigate this further, note that Unrecognized VM option 'PrintGCDateStamps' at end, FYI:
|
@vorburger ill try to take a look in the coming days |
@@ -6,8 +6,9 @@ set -ex | |||
function test_app() { | |||
local name=$1 | |||
local port="8080" | |||
local extra_args=${2:-""} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can just leave out "" (that's the default anyway)
Hello @muff1nman @rhuss I'm doing a year end clean up of my personal https://github.com/pulls and wanted to ask if you would still consider merging this very old Pull Request, or if we should forget about and close this? (This is a bulk message I'm posting to many issues to gauge what is still revelant.) |
DO NOT MERGE until #213 is fixed and this can be rebased..