Skip to content
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

Add jank client #92

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Add jank client #92

wants to merge 12 commits into from

Conversation

MathMan05
Copy link

Add jank client to the docs

Copy link
Contributor

@Puyodead1 Puyodead1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some things I think should be changed

docs/setup/clients/jank/web.md Show resolved Hide resolved
docs/setup/clients/jank/web.md Outdated Show resolved Hide resolved
```
!!! note

Jank Client defualts to opening on port 8080, if you want to change the port you'll need to change either the env variable PORT or put the port on the command you run so it'd be like node ./index.js 43
Copy link
Contributor

@Puyodead1 Puyodead1 Sep 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Jank Client defualts to opening on port 8080, if you want to change the port you'll need to change either the env variable PORT or put the port on the command you run so it'd be like node ./index.js 43
Jank Client defaults to opening on port 8080, if you want to change the port you'll need to change either the env variable PORT or put the port on the command you run, ex: `node ./index.js 43`

docs/setup/clients/jank/web.md Outdated Show resolved Hide resolved
Copy link
Member

@TheArcaneBrony TheArcaneBrony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for the most part, just have some questions.

```
!!! note

Jank Client defualts to opening on port 8080, if you want to change the port you'll need to change either the env variable PORT or put the port on the command you run, ex: `npm start ./index.js 43`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a development tool. Rewrite the build instructions to compile a static web root and provide instructions to serve with nginx or similar.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pinged you on discord so we could talk about this with most instance comunication

- Fork (Jank Client)[https://github.com/MathMan05/JankClient]
- Run the building instructions.
- Commit & push.
- Pull request & done!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a non-pull request flow for folks who don't use, or prefer not to use GitHub?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not as of now, I only have a github

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you not have discord/spacebar?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do have github/spacebar, though I don't know how exactly that's supposed to work, as neither are git providors

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sending in patches outside of git (eg. git format-patch -> git am *.patch), via any means of communication that can share one or more files :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean technically that'd work, but as with the official client
image
I don't really want to encourage that type of commit

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair, i guess im probably one of few people who would prefer to just send in patch files than whatever the hell messy thing that is github fork+pr :P

- More settings, and general bug fixes, CSS improvements or anything you think needs to be fixed/improved
- [This](https://github.com/users/MathMan05/projects/1/views/1) is a good list for things that need to be done, though it's incomplete, so if you think something is missing and you want to add it, just do it! MathMan05 is more than willing to answer your questions if you have any!
- Even if you can't code, there's stuff you can still do! Like [reporting bugs](https://github.com/MathMan05/JankClient/issues), or updating the documentation! Even if you don't help Jank Client directly, if you contributed to the Spacebar server itself, it'd still be a huge help towards Jank Client, especially if you improve/fix op codes 8/14.
- If you have an instance you can put it on Jank Clients [instance list](https://github.com/MathMan05/JankClient/blob/main/InstanceInfo.md), if you don't it'll show up on Jank Client if it's on the official instance list, but it's good if you do put it on the list as it'll let Jank Client have some additional information, and your instance will be higher on the list, even if you partially fill it out, Jank Client will merge both listings so no information is lost.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps provide a link to the official instance list?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdym

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as in, include a direct link to the official intance list instead of just saying "this thing i dont provide a link to", just use a masked link to the repo or something

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I need to rewrite for clarity, that's not what that means

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i meant more, the "if you don't it'll show up on Jank Client if it's on the official instance list," part

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just saw this, I did fix it

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