-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Oracle GoldenGate #2780
Oracle GoldenGate #2780
Conversation
sbalousek
commented
Apr 10, 2024
- Initial files to support containerized execution of Oracle GoldenGate 23
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.
It occurred to me when I finished the review that all the strings that have spaces between the letters could actually be removed without any loss of context. I figured I'd mention that here instead of going back and editing all those review comments.
Also, the reason why the copyright lines need to be 2024
is because the files themselves are new. The age of the content in those files is irrelevant in this regard.
Thanks for your time, @Djelibeybi ! |
You're welcome. :) |
@Djelibeybi - One last thing: we cannot merge until OGG 23 is released. I will let you know. |
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.
There are two very minor things I spotted but they're far from release blockers. You can decide whether or not you want to address them while the PR is still in draft mode.
- Initial files to support containerized execution of Oracle GoldenGate 23 Signed-off-by: Stephen Balousek <stephen.balousek@oracle.com>
- Clean up script execution logic Signed-off-by: Stephen Balousek <stephen.balousek@oracle.com>
- Address review comments Signed-off-by: Stephen Balousek <stephen.balousek@oracle.com>
- Additional branding changes from Product Management Signed-off-by: Stephen Balousek <stephen.balousek@oracle.com>
- Fix links to the Oracle Technology Network Signed-off-by: Stephen Balousek <stephen.balousek@oracle.com>
- Added support for PostgreSQL Signed-off-by: Stephen Balousek <stephen.balousek@oracle.com>
d9d2fb1
to
fc05a95
Compare
- Fix syntax error in `healthcheck` Signed-off-by: Stephen Balousek <stephen.balousek@oracle.com>
Hi @Djelibeybi - I am having trouble finding the error in the super linter check. Any pointers would be appreciated. |
The linting issues are being found by |
Though, now that I'm looking at it closely, I'm not sure I agree with the linter on a few things (like its suggested indentation or lack thereof). I'll take a closer look tomorrow or over the weekend and tweak the linter as necessary. If you feel the linter is being overly pedantic, let me know and I'll merge after I do a final manual check. |
- Reformat shell scripts through `shfmt -w -i 4` Signed-off-by: Stephen Balousek <stephen.balousek@oracle.com>
I simply do not see the error. The As far as I can tell, this PR failed because
|
These changes - which are critical to the GoldenGate 23ai release - passed lint checks a few weeks ago. If there is anything you can do before next week, I would really appreciate it. |
- Fix port number for NGINX health check Signed-off-by: Stephen Balousek <stephen.balousek@oracle.com>
- Run all shell scripts through `shfmt -w`, replacing leading spaces with TAB characters Signed-off-by: Stephen Balousek <stephen.balousek@oracle.com>
I thought about this more and realized TAB characters are the current requirement (per |
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.
I dislike Tab for whitespace too, so I'll adjust the SHFMT configuration accordingly so it doesn't flag that as an error in future. I'll approve and merge this in the meantime though.
Thanks a bunch, Avi. Have a fantastic weekend! |
You're welcome and likewise. |