Just-Some-Bots / MusicBot

:musical_note: The original MusicBot for Discord (formerly SexualRhinoceros/MusicBot)
https://just-some-bots.github.io/MusicBot/
MIT License
3.14k stars 2.36k forks source link

ci: refactor docker builds #2431

Closed pythoninthegrass closed 1 month ago

pythoninthegrass commented 1 month ago

After creating your pull request, tick these boxes if they are applicable to you.


Description

Related issues (if applicable)

Closes #2412

BabyBoySnow commented 1 month ago

I'll review further in the morning, (it's 3am for me as I type this) some things to note though. PR's must target the dev branch, which unfortunately led to a lot of conflicts. The editor config I won't accept, black should handle our needs well enough and is the same format d.py uses. The readme changes, should really be going to the gh-pages, not on here as the readme here is just intended for a quick overview, the gh-pages contain the guides.

itsTheFae commented 1 month ago

You should base these changes off of the dev branch instead of master. It should resolve the conflicts and will make review of the changes much easier.

As Snow mentioned, the documentation parts will need to be a separate PR into the gh-pages branch of the repo.
For that I would suggest making a file at _docs/installing/docker.md and putting the related documentation there.
We're working on updating docs at the moment, so a new file should make it easy to pull into those up coming changes.

Thanks!

BabyBoySnow commented 1 month ago

I've merged dev into this pr, which should address the merge conflicts. Other than that please look into the requested changes and suggestions from Fae and myself. Thank you!

itsTheFae commented 1 month ago

@BabyBoySnow That merge might have clobbered some edits... It may be better to make a new PR at this point.
The musicbot.service file was recently removed from dev with the latest installer updates.

If this PR is cleaned up, it must be squash-merged or it'll make a mess of commit history.

@pythoninthegrass For future PRs, I would suggest making changes in a new branch. As it stands now, some commit history may be lost when this gets squashed and you later sync your master branch with this repo.

pythoninthegrass commented 1 month ago

Removed .editorconfig and reset the readme. Do you want to continue this PR or create a new one with a feature branch on my fork?

Will open a _docs/installing/docker.md PR after this is approved.

pythoninthegrass commented 1 month ago

The editor config I won't accept, black should handle our needs well enough and is the same format d.py uses.

FYI black only covers python. Looking at the commit history here, you shouldn't be so quick to dismiss something used by much bigger projects.

itsTheFae commented 1 month ago

Lets do a new branch and a new PR. Keeps it all clean, simple, and all on-scope.
Thanks again!