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

Assembly settings #9

Merged
merged 8 commits into from
Aug 22, 2024
Merged

Assembly settings #9

merged 8 commits into from
Aug 22, 2024

Conversation

wetneb
Copy link
Member

@wetneb wetneb commented May 26, 2024

Adds the missing packaging configuration @thadguidry.

src/assembly/extension.xml Outdated Show resolved Hide resolved
src/assembly/extension.xml Show resolved Hide resolved
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/ASSEMBLY/2.0.0 http://maven.apache.org/xsd/assembly-2.0.0.xsd">
<id>openrefine-extension-archive</id>
<includeBaseDirectory>false</includeBaseDirectory>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<includeBaseDirectory>false</includeBaseDirectory>
<baseDirectory>openrefine-extension-archive</baseDirectory>
<includeBaseDirectory>true</includeBaseDirectory>

The above suggestion fixes the issue completely. That way it actually also coincides with the installation docs as well quite nicely! In our docs we were telling users themselves to make a top-level folder for the extension and then extract its contents into that. This is a better way. With my suggestion here in extension.xml , this automatically makes that top level base directory folder inside the zip and then the module/ folder shows inside that. So the user doesn't have to!

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<includeBaseDirectory>false</includeBaseDirectory>
<includeBaseDirectory>true</includeBaseDirectory>

Thanks for finding that, @thadguidry. The default base directory will be the versioned artificat ID (so something like openrefine-sample-extension-0.0.1) which I think is a good choice, rather than overriding it. openrefine-sample-extension is kind of verbose, but, whatever we choose, we should use consistently throughout.

@thadguidry
Copy link
Member

thadguidry commented May 26, 2024

Ugh, not completely fixed... hmm.

image

  1. OpenRefine starts up just fine after I extract my refine-groovy-1.0.zip directly into the webapp/extensions folder. Great!
  2. I noticed that this setup has a index.vt file in module folder ? Seems the extension is serving a page at https://127.0.0.1:3333/extension/refine-groovy

image

  1. But extension "Groovy" as an expression language now does not show up in the dropdown as before when I was bundling the extension into OpenRefine.

image

Any troubleshooting ideas? Is this still an Assembly issue or not?

Copy link
Member

@tfmorris tfmorris 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 this looks pretty close. I'll go ahead and commit Thad's suggestion (minus the baseDirectory override) and a few naming consistency fixes and we can iterate from there on any additional cleanups.

pom.xml Outdated
@@ -11,6 +11,10 @@
<description>Example extension provided for demonstration purposes</description>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<description>Example extension provided for demonstration purposes</description>
<description>Example extension provided for demonstration purposes</description>
<description>Sample OpenRefine extension provided for demonstration purposes</description>

Let's be consistent with terminology

xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/ASSEMBLY/2.0.0 http://maven.apache.org/xsd/assembly-2.0.0.xsd">
<id>openrefine-extension-archive</id>
<includeBaseDirectory>false</includeBaseDirectory>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<includeBaseDirectory>false</includeBaseDirectory>
<includeBaseDirectory>true</includeBaseDirectory>

Thanks for finding that, @thadguidry. The default base directory will be the versioned artificat ID (so something like openrefine-sample-extension-0.0.1) which I think is a good choice, rather than overriding it. openrefine-sample-extension is kind of verbose, but, whatever we choose, we should use consistently throughout.

- include the base directory in the created zip file
- make naming more consistent throughout and use properties
  to make ids & names easy to change
Copy link
Member

@tfmorris tfmorris left a comment

Choose a reason for hiding this comment

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

Let's merge this and use it as the starting point to refine things from.

@tfmorris tfmorris merged commit 21dd52f into OpenRefine:master Aug 22, 2024
2 checks passed
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