Skip to content

Conversation

@Domenic-MZS
Copy link

@Domenic-MZS Domenic-MZS commented Nov 23, 2023

Fix unescaped JAVA_X variable in fluree_start.sh for Windows Bash compatibility (#217)

This Pull Request (PR) introduces changes to the fluree_start.sh script, specifically targeting the JAVA_X variable. The main aim is to ensure compatibility with Windows Subsystem for Linux (WSL) environments like GIT BASH. This is achieved by properly escaping the JAVA_X variable. Closes issue #217.

Overview

  • Wrapped JAVA_X variable to work with Windows Path's
  • Run lua_ls LSP file format

🧪 Testing

  • Test File Modifications (didn't find a test for resources/fluree_start.sh )
  • Pass Build / Compiled

📑 Notes

The Prerequisites list was incomplete, i had to check the fluree/db repository and watch build errors to make it work, i choosed the build version v2.0.4, and the zip package is totally missing on any requirements list. It also required some base code (clojure) modifications related to the issue #213 to build. I will address that next if i get some free time.

- Escaped the JAVA_X variable to ensure proper handling in Windows WSL environments
@CLAassistant
Copy link

CLAassistant commented Nov 23, 2023

CLA assistant check
All committers have signed the CLA.

@Domenic-MZS Domenic-MZS changed the title fix unescaped JAVA_X in fluree_start.sh for windows bash compatibility Fix: unescaped JAVA_X in fluree_start.sh for windows bash compatibility (#217) Nov 23, 2023
@Domenic-MZS Domenic-MZS changed the title Fix: unescaped JAVA_X in fluree_start.sh for windows bash compatibility (#217) Fix unescaped JAVA_X variable in fluree_start.sh for Windows Bash compatibility (#217) Nov 28, 2023
@Domenic-MZS
Copy link
Author

Could I request a review from you as well on this Pull Request, @bplatz? Thank you in advance! 👾

@bplatz
Copy link
Contributor

bplatz commented Nov 29, 2023

Hi @Domenic-MZS Thanks for helping!

Is there a reason the indenting was also modified? Unsure if there is a bash indentation convention you are also suggesting here, or if this is just your personal preference.

@Domenic-MZS
Copy link
Author

Domenic-MZS commented Nov 29, 2023

Is there a reason the indenting was also modified? Unsure if there is a bash indentation convention you are also suggesting here, or if this is just your personal preference.

Hi @bplatz! Thanks for reviewing the pull request 🔎🧾. The indenting changes were automatically applied by my Neovim's configured Language Server Protocol 👾, specifically the lua_ls (lua-language-server).

I aimed to align with the bash indentation convention for better readability and consistency within the codebase 📋. The intention was to maintain uniformity and follow established best practices ✅. If there's a different preferred standard or if these modifications don't align with the codebase guidelines, I'm more than willing to make the adjustments accordingly ✍🏼.

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