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

Update README.md for v0.2.0 #24

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Update README.md for v0.2.0 #24

merged 1 commit into from
Jun 12, 2024

Conversation

dmikurube
Copy link
Member

No description provided.

@dmikurube dmikurube requested a review from a team as a code owner June 12, 2024 08:46
@dmikurube dmikurube force-pushed the README-v0.2.0 branch 4 times, most recently from c024f83 to b96947d Compare June 12, 2024 08:54
Copy link
Member

@hiroyuki-sato hiroyuki-sato left a comment

Choose a reason for hiding this comment

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

LGTM👍 (Left one minor comment)


artifact "org.embulk:embulk-input-postgresql:0.13.2"
artifact group: "org.embulk", name: "embulk-input-s3", version: "0.6.0"

// It downloads jruby-complete-9.1.15.0.jar, and set the "jruby" Embulk System Property in "embulk.properties".
Copy link
Member

Choose a reason for hiding this comment

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

Minor Adding the location of download JRuby file may useful. what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Adding some explanation in README? Or as a kind of feature?

Copy link
Member

Choose a reason for hiding this comment

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

It meant in README.

// JRuby install in the xxx directory

Copy link
Member Author

Choose a reason for hiding this comment

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

The path would be written in embulk.properties. I do not want to make it "documented" because documenting it would chain our future changes.

Copy link
Member

Choose a reason for hiding this comment

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

I see, Thanks!

@dmikurube
Copy link
Member Author

Thanks anyway! Let me merge this.

@dmikurube dmikurube merged commit 9e2a00b into main Jun 12, 2024
4 checks passed
@dmikurube dmikurube deleted the README-v0.2.0 branch June 12, 2024 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants