chore: update devnotes #97
Conversation
tchow-zlai
requested review from
nikhil-zlai,
varant-zlai,
ken-zlai,
piyush-zlai and
chewy-zlai
WalkthroughThe changes primarily focus on enhancing the Changes
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
devnotes.md (3)
32-38: Add specific Python version and installation commandsWhile the Java and Scala versions are clearly specified, the Python version requirement is missing. Additionally, consider adding the exact
asdfcommands for installing these versions.### Install appropriate java, scala, and python versions Recommend you do this through a runtime version manager. [asdf](https://asdf-vm.com/) recommended since it works for most all languages. - Use scala`2.13` and java `corretto-17` for the Zipline Chronon distribution. - Use scala `2.12.18` and java `corretto-8` for the OSS Chronon distribution. + +Example installation commands: +```bash +# Install plugins +asdf plugin add java +asdf plugin add scala +asdf plugin add python + +# Install versions +asdf install java corretto-17 +asdf install scala 2.13 +asdf install python 3.x.x # specify required version +```
49-52: Fix formatting and punctuationMinor formatting improvements needed:
-Under Intellij IDEA > Settings > Editor > Code Style > Scala enable `scalafmt`. +Under Intellij IDEA > Settings > Editor > Code Style > Scala, enable `scalafmt`.🧰 Tools
🪛 LanguageTool
[uncategorized] ~51-~51: Possible missing comma found.
Context: ...IDEA > Settings > Editor > Code Style > Scala enablescalafmt. - Follow the steps b...(AI_HYDRA_LEO_MISSING_COMMA)
57-57: Fix grammar in instructionsMinor grammar correction needed:
-Set the following [java arguments](https://stackoverflow.com/questions/72724816/running-unit-tests-with-spark-3-3-0-on-java-17-fails-with-illegalaccesserror-cl) by copy pasting into the run configuration arguments list: +Set the following [java arguments](https://stackoverflow.com/questions/72724816/running-unit-tests-with-spark-3-3-0-on-java-17-fails-with-illegalaccesserror-cl) by copying and pasting into the run configuration arguments list:🧰 Tools
🪛 LanguageTool
[grammar] ~57-~57: Did you mean “copying and pasting”?
Context: ...17-fails-with-illegalaccesserror-cl) by copy pasting into the run configuration arguments li...(COPY_PASTE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
intellij_unit_test_1.pngis excluded by!**/*.pngintellij_unit_test_2.pngis excluded by!**/*.pngintellij_unit_test_3.pngis excluded by!**/*.png
📒 Files selected for processing (1)
devnotes.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
devnotes.md
[uncategorized] ~51-~51: Possible missing comma found.
Context: ...IDEA > Settings > Editor > Code Style > Scala enable scalafmt. - Follow the steps b...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~57-~57: Did you mean “copying and pasting”?
Context: ...17-fails-with-illegalaccesserror-cl) by copy pasting into the run configuration arguments li...
(COPY_PASTE)
🪛 Markdownlint (0.35.0)
devnotes.md
55-55: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
73-73: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
76-76: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
| - Follow the steps below to configure unit tests in intellij: | ||
|
|
||
| Run > Edit Configurations | ||
|  |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add alt text to images for accessibility
The images lack alt text, which is important for accessibility. Please add descriptive alt text to each image:
-
+
-
+
-
+Also applies to: 73-73, 76-76
🧰 Tools
🪛 Markdownlint (0.35.0)
55-55: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
devnotes.md (4)
35-37: Clarify Scala version compatibilityThe instructions mention using Scala "2.12" for Zipline distribution but "2.12.18" for OSS distribution. Consider being more specific about the exact version for Zipline distribution to avoid confusion.
-Use scala`2.12` and java `corretto-17` for the Zipline Chronon distribution. +Use scala `2.12.18` and java `corretto-17` for the Zipline Chronon distribution.
57-72: Add explanation for Java argumentsWhile the link to StackOverflow is helpful, it would be beneficial to briefly explain why these Java arguments are necessary (i.e., for Spark compatibility with Java 17).
Add a brief explanation before the arguments:
-Set the following [java arguments](https://stackoverflow.com/questions/72724816/running-unit-tests-with-spark-3-3-0-on-java-17-fails-with-illegalaccesserror-cl) by copy pasting into the run configuration arguments list: +Set the following Java arguments to enable Spark compatibility with Java 17 by copying and pasting into the run configuration arguments list. These arguments allow Spark to access necessary Java internal APIs:🧰 Tools
🪛 LanguageTool
[grammar] ~57-~57: Did you mean “copying and pasting”?
Context: ...17-fails-with-illegalaccesserror-cl) by copy pasting into the run configuration arguments li...(COPY_PASTE)
Line range hint
316-321: Add warning about release timingConsider adding a note about the best time to perform releases (e.g., during low-traffic periods) and estimated time for completion.
Add a warning before the release steps:
# Chronon Release Process +> **Note**: Release process typically takes 30-45 minutes to complete. Consider performing releases during low-traffic periods to minimize impact. + ## Publishing all the artifacts of Chronon🧰 Tools
🪛 LanguageTool
[grammar] ~57-~57: Did you mean “copying and pasting”?
Context: ...17-fails-with-illegalaccesserror-cl) by copy pasting into the run configuration arguments li...(COPY_PASTE)
🪛 Markdownlint (0.35.0)
85-85: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
55-55: null
Images should have alternate text (alt text)(MD045, no-alt-text)
73-73: null
Images should have alternate text (alt text)(MD045, no-alt-text)
76-76: null
Images should have alternate text (alt text)(MD045, no-alt-text)
Line range hint
441-463: Enhance the zpush function with error handlingThe zpush function could be improved with better error handling and validation.
Consider enhancing the function:
function zpush() { if [ $# -eq 0 ]; then echo "Error: Please provide a commit message." return 1 fi local commit_message="$1" + local exit_code=0 + + echo "Running compilation and formatting checks..." + + # Run each command and capture its status + sbt compile + exit_code=$((exit_code + $?)) + + sbt scalafixAll + exit_code=$((exit_code + $?)) + + sbt scalafmt + exit_code=$((exit_code + $?)) + + if [ $exit_code -ne 0 ]; then + echo "Error: Compilation or formatting failed. Please fix the issues before committing." + return 1 + fi - sbt compile && \ - sbt scalafixAll && \ - sbt scalafmt && \ git add -u && \ git commit -m "$commit_message" && \ git push if [ $? -eq 0 ]; then echo "Successfully compiled, formatted, committed, and pushed changes." else echo "An error occurred during the process." + return 1 fi }🧰 Tools
🪛 LanguageTool
[grammar] ~57-~57: Did you mean “copying and pasting”?
Context: ...17-fails-with-illegalaccesserror-cl) by copy pasting into the run configuration arguments li...(COPY_PASTE)
🪛 Markdownlint (0.35.0)
85-85: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
55-55: null
Images should have alternate text (alt text)(MD045, no-alt-text)
73-73: null
Images should have alternate text (alt text)(MD045, no-alt-text)
76-76: null
Images should have alternate text (alt text)(MD045, no-alt-text)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
devnotes.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
devnotes.md
[grammar] ~57-~57: Did you mean “copying and pasting”?
Context: ...17-fails-with-illegalaccesserror-cl) by copy pasting into the run configuration arguments li...
(COPY_PASTE)
🪛 Markdownlint (0.35.0)
devnotes.md
55-55: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
73-73: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
76-76: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
🔇 Additional comments (1)
devnotes.md (1)
55-76: Add alt text to images for better accessibility
This was previously mentioned in an earlier review. Please add descriptive alt text to all images to improve accessibility.
🧰 Tools
🪛 LanguageTool
[grammar] ~57-~57: Did you mean “copying and pasting”?
Context: ...17-fails-with-illegalaccesserror-cl) by copy pasting into the run configuration arguments li...
(COPY_PASTE)
🪛 Markdownlint (0.35.0)
55-55: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
73-73: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
76-76: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
## Summary - Update devnotes based on onboarding doc. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Enhanced setup instructions for the Chronon project. - Expanded prerequisites section with environment variable configurations. - Clarified installation instructions for Thrift, emphasizing version compatibility. - Added guidance for installing Java, Scala, and Python using `asdf`. - Restructured IntelliJ configuration instructions for improved clarity. - Updated troubleshooting section with commands for project cleaning and assembly. - Elaborated on the release process for artifact publishing and code pushing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - Update devnotes based on onboarding doc. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Enhanced setup instructions for the Chronon project. - Expanded prerequisites section with environment variable configurations. - Clarified installation instructions for Thrift, emphasizing version compatibility. - Added guidance for installing Java, Scala, and Python using `asdf`. - Restructured IntelliJ configuration instructions for improved clarity. - Updated troubleshooting section with commands for project cleaning and assembly. - Elaborated on the release process for artifact publishing and code pushing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - Update devnotes based on onboarding doc. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Enhanced setup instructions for the Chronon project. - Expanded prerequisites section with environment variable configurations. - Clarified installation instructions for Thrift, emphasizing version compatibility. - Added guidance for installing Java, Scala, and Python using `asdf`. - Restructured IntelliJ configuration instructions for improved clarity. - Updated troubleshooting section with commands for project cleaning and assembly. - Elaborated on the release process for artifact publishing and code pushing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - Update devnotes based on onboarding doc. ## Cheour clientslist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Enhanced setup instructions for the Chronon project. - Expanded prerequisites section with environment variable configurations. - Clarified installation instructions for Thrift, emphasizing version compatibility. - Added guidance for installing Java, Scala, and Python using `asdf`. - Restructured IntelliJ configuration instructions for improved clarity. - Updated troubleshooting section with commands for project cleaning and assembly. - Elaborated on the release process for artifact publishing and code pushing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Checklist
Summary by CodeRabbit
asdf.