Skip to content

jmeter.apache.org#2730

Merged
jhheider merged 8 commits into
pkgxdev:mainfrom
and-ri:new/jmeter.apache.org
Aug 3, 2023
Merged

jmeter.apache.org#2730
jhheider merged 8 commits into
pkgxdev:mainfrom
and-ri:new/jmeter.apache.org

Conversation

@and-ri
Copy link
Copy Markdown
Contributor

@and-ri and-ri commented Aug 2, 2023

closes #698

@mxcl
Copy link
Copy Markdown
Contributor

mxcl commented Aug 2, 2023

can we prefer eg. apache.org/jmeter for such things.

I know it's not strictly the homepage, but that's not the primary reason for our namespacing scheme.

I'm trying to keep the root namespace a little clearer.

@and-ri
Copy link
Copy Markdown
Contributor Author

and-ri commented Aug 3, 2023

can we prefer eg. apache.org/jmeter for such things.

I know it's not strictly the homepage, but that's not the primary reason for our namespacing scheme.

I'm trying to keep the root namespace a little clearer.

Yes sure

@and-ri and-ri marked this pull request as ready for review August 3, 2023 08:02
@jhheider jhheider merged commit f82da9d into pkgxdev:main Aug 3, 2023
Comment thread projects/jmeter.apache.org/package.yml Outdated
openjdk.org: '*'
runtime:
env:
JAVA_HOME: "{{prefix}}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems wrong, the user can set this, we are overriding it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm 🤔

perhaps it would be correct to check the existence of a variable and assign a value if it does not exist?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is it set? Add comments pls

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

JAVA_HOME just resolve problem

The operation couldn’t be completed. Unable to locate a Java Runtime.
Please visit http://www.java.com/ for information on installing Java.

https://github.com/teaxyz/pantry/actions/runs/5743363132/job/15567693928

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Or maybe we can add JAVA_HOME in test section and add README.md with information that JAVA_HOME must be set in env?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't JAVA_HOME be ${{deps.openjdk.org.prefix}}, and exported from that package? Per my limited recollection of configuring Java. @ABevier was a Javanista: any thoughts on this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the final version is {{deps.openjdk.org.prefix}}

70957af

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Right. But shouldn't that be in the runtime of openjdk.org, not packages that depend on it?

Copy link
Copy Markdown
Contributor Author

@and-ri and-ri Aug 4, 2023

Choose a reason for hiding this comment

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

Right. But shouldn't that be in the runtime of openjdk.org, not packages that depend on it?

Yes, I had thoughts of doing this too. I made changes to this pull request
#2727

@and-ri and-ri deleted the new/jmeter.apache.org branch September 20, 2023 17:43
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.

+jmeter (457/548)

3 participants