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

v6.1.0 #337

Merged
merged 42 commits into from
Aug 29, 2018
Merged

v6.1.0 #337

merged 42 commits into from
Aug 29, 2018

Conversation

Dougley
Copy link
Member

@Dougley Dougley commented Aug 18, 2018

Please refer to the changelog for a full list of changes

Dougley and others added 30 commits July 9, 2018 10:16
this wont require the entire directory just to use 1 driver
docgen interferes with this, since running only docgen doesnt load logger
according to the documentation, this should allow this job to run on tags as well as on new commits
this command throws fatal typeerrors inside the imgflipper module, replacing this soon with something hopefully not terrible
docs didnt like it
* Small changes to make things more understandable

* Update en-EN.json

* Update en-EN.json
@Dougley Dougley requested review from linuswillner and a team as code owners August 18, 2018 16:50
Copy link
Contributor

@linuswillner linuswillner left a comment

Choose a reason for hiding this comment

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

Some nitpicks here. I also need to make a small amend to the documentation to reflect the changes to the meme command.

@@ -39,38 +39,38 @@ module.exports = {
}
} else {
resolveTracks(suffix).then(tracks => {
Copy link
Contributor

Choose a reason for hiding this comment

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

For improved code quality and readability, perhaps name the callback variable trackInfo and cast trackInfo.tracks to a constant. This is a nitpick, but tracks.tracks is not very descriptive and more confusing than anything else.

@@ -36,40 +36,38 @@ module.exports = {
}
} else {
resolveTracks(suffix).then(tracks => {
Copy link
Contributor

Choose a reason for hiding this comment

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

My suggestion for join-voice applies here as well.

@@ -35,9 +35,10 @@ module.exports = {
}
})
} else {
msg.channel.createMessage(`<@${msg.author.id}>, ${suffix}: This word is so screwed up, even Urban Dictionary doesn't have it in its database`)
msg.channel.createMessage(`<@${msg.author.id}>, ${suffix}: This word is so screwed up, even UrbanDictionary doesn't have it in its database`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Name mangling issue here. According to Wikipedia, the name is in fact Urban Dictionary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hah, accidentally changed this with my last commit, will fix.

@@ -64,7 +64,8 @@ module.exports = {
sendToES({
type: 'command',
cmd: opts.cmd,
full: opts.cmd + ' ' + opts.opts,
// full: opts.cmd + ' ' + opts.opts,
Copy link
Contributor

Choose a reason for hiding this comment

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

Commented out code should not be left in the production build.

src/internal/version-check.js Show resolved Hide resolved
Linus Willner and others added 3 commits August 18, 2018 23:39
* Fixed the command documentation generator never going out of temp state
* Improved documentation for certain commands
There was a lack of parity between the CI and development environment scripts for command docgen, causing a condition where the command documentation would fail to build properly in development.
@zaza7 zaza7 self-requested a review August 18, 2018 23:14
Dougley and others added 3 commits August 28, 2018 20:45
This commit fixes current code to work with Lavalink v3 as well as with FFmpeg.
Copy link
Contributor

@zaza7 zaza7 left a comment

Choose a reason for hiding this comment

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

This PR gets a 👍 from me, no problems that are obvious.

@Dougley Dougley merged commit ea1f1fa into master Aug 29, 2018
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.

4 participants