-
Notifications
You must be signed in to change notification settings - Fork 2
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
[enhancement/readme] Improved readme #48
base: develop
Are you sure you want to change the base?
Conversation
@@ -2,14 +2,39 @@ | |||
## Prerequisites | |||
- Java 21 | |||
- Docker | |||
## Used technologies |
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.
- Rename to "Technology Stack".
- Merge this section with
Prerequisites
section; - Add a note in which you inform that Docker is mandatory.
- Add version number of each technology used.
|
||
# Structure of the project | ||
There are two projects, JShellAPI and JShellWrapper | ||
JShellAPI is a REST API, and whenever some code is received, it will create a session, by creating a docker container, which will run JShellWrapper inside and then execute the given code in JShellWrapper. | ||
There are two projects, JShellAPI and JShellWrapper. |
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 would be nice to make JShellAPI and JShellWrapper words as links.
- it's better to say
This project is composed of 2 sub-projects
, both are Backend services (as indicated in the name of the repo) - I highly suggest to share the source of the diagram so we can update anytime we want.
JShellAPI is a REST API, and whenever some code is received, it will create a session, by creating a docker container, which will run JShellWrapper inside and then execute the given code in JShellWrapper. | ||
There are two projects, JShellAPI and JShellWrapper. | ||
|
||
JShellAPI is a REST API, where whenever some code is received, it will create a session, by creating a docker container, which will run JShellWrapper inside and then execute the given code. |
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.
- Add a title here to inform that this section is about the 1st sub-project;
- Bring any relevant information to JShellApi project under this section;
- It would be better to make
REST API
keyword as a link to Restful API website; - We need to explain to explain that REST API (JShellApi) is the entry point to JShellWrapper and not the opposite.
- We need to explain why we don't allow executing the code within the REST API service (reasons or links);
|
||
JShellAPI is a REST API, where whenever some code is received, it will create a session, by creating a docker container, which will run JShellWrapper inside and then execute the given code. | ||
|
||
![JShellAPI diagram](https://github.com/user-attachments/assets/c566b8c0-cf81-4a56-8a0a-5715c33105a7) |
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.
- Why not putting the diagram img within the repo ?
- There is no mention of a scenario for using an existing session;
- Why there is no mention to session on the diagram?
|
||
![JShellAPI diagram](https://github.com/user-attachments/assets/c566b8c0-cf81-4a56-8a0a-5715c33105a7) | ||
|
||
There are three unrelated ways to run JShellPlaygroundBackend: |
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 would be better to say There are 3 ...
* * On `Windows Powershell`, use `$env:evalTimeoutSeconds=15 && $env:sysOutCharLimit=1024` | ||
* * On `Linux Shell`, use `export evalTimeoutSeconds=15 && export sysOutCharLimit=1024` | ||
* Run `./gradlew JShellWrapper:run` | ||
* Alternatively, run: |
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'd suggest to delete this part :
* Set the environment variable `evalTimeoutSeconds=15` and `sysOutCharLimit=1024`
* * On `Windows CMD`, use `set evalTimeoutSeconds=15 && set sysOutCharLimit=1024`
* * On `Windows Powershell`, use `$env:evalTimeoutSeconds=15 && $env:sysOutCharLimit=1024`
* * On `Linux Shell`, use `export evalTimeoutSeconds=15 && export sysOutCharLimit=1024`
* Run `./gradlew JShellWrapper:run`
* Alternatively, run:
curl --request POST --url http://localhost:8080/jshell/eval/test --data '2+2' | ||
``` | ||
|
||
### How to use JShellApi ? |
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.
Please place this under a JShellAPi section
curl --request POST --url http://localhost:8080/jshell/eval/test --data '2+2' | ||
``` | ||
|
||
### How to use JShellApi ? | ||
See [JShellAPI README](JShellAPI/README.MD) |
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 would be better to bring the content here and add note to inform that it's NOT updated
@@ -45,5 +70,12 @@ JShellAPI is a REST API, and whenever some code is received, it will create a se | |||
-application.yaml | |||
``` | |||
|
|||
## How to use JShellApi ? | |||
### Try JShell API |
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.
Place this section under JShellApi section
Pull-request
Changes
Closes Issue: #44
Description
Replace this sentence with general description of what your Pull Request does.