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

Update #693

Closed
wants to merge 49 commits into from
Closed

Update #693

wants to merge 49 commits into from

Conversation

hwangsihu
Copy link
Contributor

No description provided.

@hwangsihu hwangsihu force-pushed the main branch 2 times, most recently from c162cf4 to 210328c Compare August 21, 2024 04:36
@infnibor
Copy link
Contributor

Bad lavalink config

@hwangsihu
Copy link
Contributor Author

What?

@infnibor
Copy link
Contributor

infnibor commented Aug 21, 2024

What?

If you want to edit it, read the documentation carefully, arl is commented out by default and so are its options, the Android client has long been broken and is not used instead of Android Test Suite, and the fact that in the YouTube plugin repo you have the clients listed in this order does not mean that you have to set it that way, you set it according to your own applications e.t.c.

@LucasB25
Copy link
Collaborator

Your PR will remain open for a while, lavalink changes are not officially published yet

@LucasB25 LucasB25 self-requested a review August 21, 2024 13:40
@LucasB25 LucasB25 self-assigned this Aug 21, 2024
@hwangsihu
Copy link
Contributor Author

ok

LucasB25 and others added 5 commits August 21, 2024 17:06
Bumps [docker/build-push-action](https://github.com/docker/build-push-action) from 5 to 6.
- [Release notes](https://github.com/docker/build-push-action/releases)
- [Commits](docker/build-push-action@v5...v6)

---
updated-dependencies:
- dependency-name: docker/build-push-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
…/build-push-action-6

Bump docker/build-push-action from 5 to 6
@LucasB25
Copy link
Collaborator

@hwangsihu Please split its modification lavalink to another PR

@hwangsihu
Copy link
Contributor Author

@hwangsihu수정된 lavalink를 다른 PR로 분할해주세요.

What do you mean

@infnibor
Copy link
Contributor

@hwangsihu수정된 lavalink를 다른 PR로 분할해주세요.

What do you mean
Changes regarding incorrect version of skybot plugin in other new PR

@hwangsihu
Copy link
Contributor Author

Do you mean #696?

@infnibor
Copy link
Contributor

Do you mean #696?

Yea

@hwangsihu
Copy link
Contributor Author

hwangsihu commented Aug 25, 2024

It's already at 1.7.0, so should I revert it back to the original?

@infnibor
Copy link
Contributor

It's already at 1.7.0, so should I revert it back to the original?

Fix by #697 - less translation, and you have the correction right away, the issue of PR consolidation.

@hwangsihu
Copy link
Contributor Author

Ok

@hwangsihu
Copy link
Contributor Author

Is it okay to commit like this?

@hwangsihu
Copy link
Contributor Author

Is there a reason to be set in config.ts? I only use it as YouTube, so I will be pushed here every time I commit something.

@LucasB25
Copy link
Collaborator

do as you did create problems in the requests

@infnibor
Copy link
Contributor

infnibor commented Aug 25, 2024

I would recommend closing this PR, and bot info unnecessarily throwing away the work of the Appu, regarding downloading variables from language files, making millions of unnecessary commits - you test locally at your place and make any corrections, because then over 20 commits go to the repo, and most of the changes are pointless if you don't know the meaning and documentation

@infnibor
Copy link
Contributor

In short, the clearer it is, the better for the recipient - a pending commit has a description of what has been changed and, of course, sometimes it happens that corrections are needed, but then either you amend the commit or at least describe it properly, because then 10 commits mean ".". I have no complaints, I just try to make the changes clear even for other contributors, etc.

@hwangsihu hwangsihu closed this Aug 25, 2024
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