Skip to content

Feature/doobie with status#106

Merged
salamonpavel merged 68 commits into
masterfrom
feature/doobie-with-status
Jan 3, 2024
Merged

Feature/doobie with status#106
salamonpavel merged 68 commits into
masterfrom
feature/doobie-with-status

Conversation

@salamonpavel

@salamonpavel salamonpavel commented Dec 8, 2023

Copy link
Copy Markdown
Contributor

In this PR, the below listed issues have been addressed:

I am aware that this pull request contains a substantial number of changes. I understand that this can make the review process more challenging and time-consuming, and I appreciate your efforts in ensuring our codebase maintains its high quality.

I am fully aware that the volume of changes may be overwhelming, and it's important to me that everyone on the team feels comfortable with these updates. To that end, I am more than willing to walk anyone through the code changes, explain the reasoning behind each modification, and answer any questions you may have.

Please feel free to reach out to me at your convenience if you would like a walkthrough of the code or if you have any questions or concerns.

Please review the changes and provide your feedback.

Closes #11
Closes #51
Closes #99
Closes #104
Closes #105

@jakipatryk jakipatryk left a comment

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.

I reviewed only part of the PR, but anyway I don't feel I know enough internals of fa-db to approve.

Comment thread .github/workflows/build.yml Outdated
Comment thread core/src/main/scala/za/co/absa/fadb/DBEngine.scala Outdated
Comment thread core/src/main/scala/za/co/absa/fadb/DBEngine.scala Outdated
Comment thread core/src/main/scala/za/co/absa/fadb/DBEngine.scala Outdated
@lsulak

lsulak commented Dec 27, 2023

Copy link
Copy Markdown
Collaborator

File /home/runner/work/fa-db/fa-db/examples/src/main/scala/za/co/absa/fadb/examples/enceladus/DatasetSchema.scala still doesn't have licence header according to the CI

@lsulak

lsulak commented Dec 27, 2023

Copy link
Copy Markdown
Collaborator

Jacoco doesn't seem to work here, can you take a look @miroslavpojer please? Perhaps this is something you encountered before

@salamonpavel

Copy link
Copy Markdown
Contributor Author

File /home/runner/work/fa-db/fa-db/examples/src/main/scala/za/co/absa/fadb/examples/enceladus/DatasetSchema.scala still doesn't have licence header according to the CI

Seems that the license formatting was off. The license check passes now.

jakipatryk
jakipatryk previously approved these changes Jan 3, 2024

@jakipatryk jakipatryk left a comment

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.

LGTM (just read the code), though as I said I don't have much fa-db internals knowledge.

@github-actions

github-actions Bot commented Jan 3, 2024

Copy link
Copy Markdown

JaCoCo code coverage report - scala 2.13.12

Total Project Coverage 63.49% 🍏
File Coverage [63.49%]
LettersCase.scala 100% 🍏
ExplicitNamingRequired.scala 100% 🍏
NamingConvention.scala 100% 🍏
FunctionStatusWithData.scala 100% 🍏
StandardStatusHandling.scala 100% 🍏
FunctionStatus.scala 100% 🍏
NamingException.scala 100% 🍏
StatusException.scala 100% 🍏
DBFunctionFabric.scala 97.59% 🍏
DBSchema.scala 93.88% 🍏
SnakeCaseNaming.scala 89.29% 🍏
AsIsNaming.scala 72.22%
DBEngine.scala 20%
DBFunction.scala 7.11%
Query.scala 0%
UserDefinedStatusHandling.scala 0%

@salamonpavel salamonpavel merged commit c91623c into master Jan 3, 2024
@salamonpavel salamonpavel deleted the feature/doobie-with-status branch January 3, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants