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

Certgen & QOL changes #19

Closed
wants to merge 2 commits into from
Closed

Certgen & QOL changes #19

wants to merge 2 commits into from

Conversation

jacksonisiah
Copy link
Contributor

Up to you if you want to merge this. Certgen was working last time I used it which was about a month ago :)

@cmyui
Copy link
Member

cmyui commented Oct 17, 2020

Hey, there seems to be some sort of difference in the format from this cert to akatsuki/ripple/etc.. i think this may be the same as the ainu certificate, which seems to work.. sometimes? lol

once i figure out the differences, i might add a certgen (but i'm also not sure about how the osu team will feel about it xd)

@cmyui cmyui closed this Oct 17, 2020
@cmyui
Copy link
Member

cmyui commented Jan 9, 2023

Hey, it's been a while, but if you'd like to PR the certgen into /tools I'd be willing to merge it - it's pretty useful for those who want to run a server locally.

(Generally we suggest prod users to use cloudflare flex to sidestep tls entirely on the server-side)

@cmyui
Copy link
Member

cmyui commented Jan 9, 2023

one point for that pr if you go through with it - ppy.sh should become a variable now that we have the -devserver flag in the osu! stable client

@7mochi
Copy link
Contributor

7mochi commented Jan 9, 2023

Hey, it's been a while, but if you'd like to PR the certgen into /tools I'd be willing to merge it - it's pretty useful for those who want to run a server locally.

(Generally we suggest prod users to use cloudflare flex to sidestep tls entirely on the server-side)

Wouldn't it be better to use mkcert for localhost certs?

@tsunyoku
Copy link
Contributor

tsunyoku commented Jan 9, 2023

Hey, it's been a while, but if you'd like to PR the certgen into /tools I'd be willing to merge it - it's pretty useful for those who want to run a server locally.
(Generally we suggest prod users to use cloudflare flex to sidestep tls entirely on the server-side)

Wouldn't it be better to use mkcert for localhost certs?

i agree, mkcert has had a more intuitive process for me

@cmyui
Copy link
Member

cmyui commented Jan 9, 2023

Hey, it's been a while, but if you'd like to PR the certgen into /tools I'd be willing to merge it - it's pretty useful for those who want to run a server locally.
(Generally we suggest prod users to use cloudflare flex to sidestep tls entirely on the server-side)

Wouldn't it be better to use mkcert for localhost certs?

Perhaps - I had some issues with generating one for local dev (perhaps an issue with my usage), and have always fallen back to using openssl directly. If you'd like to propose a PR for it, I'm open to the best suggestion!

@cmyui
Copy link
Member

cmyui commented Jan 9, 2023

Might also be useful to include a small document to define the process of setting up a local server, including hosts file redirection and such. Could be a separate PR or together as one.

@7mochi
Copy link
Contributor

7mochi commented Jan 9, 2023

I was thinking of adding it also in #320 but wouldn't the README be too bloated? I think the best thing would be to move some of those instructions to github wiki

@NiceAesth
Copy link
Member

I've spoken with Jackson and he is open to fixing this PR up. (I think documentation should be a separate PR btw since translations and such)

@cmyui
Copy link
Member

cmyui commented Jan 9, 2023

I was thinking of adding it also in #320

Keep PRs small and specific, this PR is already a bit large for my liking (e.g. I think we could have split up the ssl removal & and the docker setup, and reviewed those separately).

Big PRs are much more difficult to keep up to date while master is worked on, and also harder to review.

I've had to learn this lesson multiple times, the hard way, because I couldn't merge my changes anymore as it was far too much work; some examples:

I've found much more success in breaking down a large task into smaller subcomponents; a good example is my recent work on refactoring SQL queries into repositories;

Work is also much more parallelizable this way - if you have a large chunk of work that you've successfully split up (like this), multiple people can work on it at the same time (@yo-ru has been helping me out!).

@minisbett
Copy link
Contributor

I would disagree with renaming ext to external

@NiceAesth
Copy link
Member

I would disagree with renaming ext to external

There is no point in reviewing a PR from 2020 (ext did not exist back then)

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.

6 participants