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

Chore: command's endpoints #25630

Merged
merged 5 commits into from
Jun 3, 2022
Merged

Chore: command's endpoints #25630

merged 5 commits into from
Jun 3, 2022

Conversation

ggazzo
Copy link
Member

@ggazzo ggazzo commented May 25, 2022

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

@ggazzo ggazzo requested a review from a team as a code owner May 25, 2022 13:52
@ggazzo ggazzo added the eng day label May 25, 2022
@ggazzo ggazzo changed the title Chore: SlashCommand Chore: SlashCommand endpoints May 25, 2022
@ggazzo ggazzo changed the title Chore: SlashCommand endpoints Chore: command's endpoints May 25, 2022
});
slashCommands.add(
'shrug',
(_command: 'shrug', params, item): void => {
Copy link
Contributor

Choose a reason for hiding this comment

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

An anonymous function is harder to debug.

});
slashCommands.add(
'tableflip',
(_command, params, item): void => {
Copy link
Contributor

Choose a reason for hiding this comment

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

An anonymous function is harder to debug.

let channel = params.trim();
slashCommands.add(
'archive',
function Archive(_command, params, item): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nesting this function declaration as a parameter expression damages readability, as evidenced by the indentation increase.

});
slashCommands.add(
'bridge',
function Bridge(_command, stringParams, item): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nesting this function declaration as a parameter expression damages readability, as evidenced by the indentation increase.

regex.lastIndex++;
slashCommands.add(
'create',
function Create(_command: 'create', params, item): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nesting this function declaration as a parameter expression damages readability, as evidenced by the indentation increase.

slashCommands.add(
'create',
function Create(_command: 'create', params, item): void {
function getParams(str: string): string[] {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick Maybe move it to the outer scope.

const user = Users.findOneById(userId);
slashCommands.add(
'help',
function Help(_command, _params, item): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nesting this function declaration as a parameter expression damages readability, as evidenced by the indentation increase.

const userId = Meteor.userId() as string;
slashCommands.add(
'status',
function Status(_command, params, item): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nesting this function declaration as a parameter expression damages readability, as evidenced by the indentation increase.

const userId = Meteor.userId() as string;
slashCommands.add(
'status',
function Status(_command: 'status', params, item): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nesting this function declaration as a parameter expression damages readability, as evidenced by the indentation increase.

handleError(err);
slashCommands.add(
'topic',
function Topic(_command: 'topic', params, item): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nesting this function declaration as a parameter expression damages readability, as evidenced by the indentation increase.

});
slashCommands.add(
'topic',
function Topic(_command: 'topic', params, item): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nesting this function declaration as a parameter expression damages readability, as evidenced by the indentation increase.

let room;
slashCommands.add(
'unarchive',
function Unarchive(_command: 'unarchive', params, item): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nesting this function declaration as a parameter expression damages readability, as evidenced by the indentation increase.

@ggazzo ggazzo merged commit 08b7b22 into develop Jun 3, 2022
@ggazzo ggazzo deleted the chore/SlashCommands branch June 3, 2022 15:58
@ggazzo
Copy link
Member Author

ggazzo commented Jun 3, 2022

if I do it the other way, it will force me to type the functions, which is a dynamic type, and it looks horrible @tassoevan

gabriellsh added a commit that referenced this pull request Jun 6, 2022
* origin/develop: (45 commits)
  [FIX] Thread Message Preview (#25709)
  [FIX] Bump meteor-node-stubs to version 1.2.3 (#25669)
  [IMPROVE] Refactor + unit tests for federation-v2 (#25680)
  [FIX] user status Offline misnamed as Invisible in Custom Status edit dropdown menu (#24796)
  Chore: Messages raw model rewrite to ts (#25761)
  Chore: migrate katex to ts (#25501)
  Chore: AutoTranslate contextualBar rewrite  (#25751)
  Chore: Replace AnnouncementModal in favor of GenericModal (#25752)
  Chore: Keyboard shortcuts contextualBar rewrite (#25753)
  Chore: Prune Messages contextualBar rewrite (#25757)
  Chore: add Ajv JSON Schema to api/v1 (#25601)
  Update package.json (#25755)
  Update CODEOWNERS
  Chore: remove duplicated NotFoundPage.js (#25749)
  Chore: command's endpoints (#25630)
  Chore: Fix incorrect checksum for agenda package (cause of breaking develop builds) (#25741)
  Chore: Remove duplicate checksumBehavior key from yarn file (#25730)
  [FIX] Custom emoji reaction size (#25393)
  Chore: Test for department screen (#25696)
  Chore: Taking out Blaze from routes with `MainLayout`  (#25697)
  ...
gabriellsh added a commit that referenced this pull request Jun 6, 2022
* origin/develop: (26 commits)
  [FIX] Thread Message Preview (#25709)
  [FIX] Bump meteor-node-stubs to version 1.2.3 (#25669)
  [IMPROVE] Refactor + unit tests for federation-v2 (#25680)
  [FIX] user status Offline misnamed as Invisible in Custom Status edit dropdown menu (#24796)
  Chore: Messages raw model rewrite to ts (#25761)
  Chore: migrate katex to ts (#25501)
  Chore: AutoTranslate contextualBar rewrite  (#25751)
  Chore: Replace AnnouncementModal in favor of GenericModal (#25752)
  Chore: Keyboard shortcuts contextualBar rewrite (#25753)
  Chore: Prune Messages contextualBar rewrite (#25757)
  Chore: add Ajv JSON Schema to api/v1 (#25601)
  Update package.json (#25755)
  Update CODEOWNERS
  Chore: remove duplicated NotFoundPage.js (#25749)
  Chore: command's endpoints (#25630)
  Chore: Fix incorrect checksum for agenda package (cause of breaking develop builds) (#25741)
  Chore: Remove duplicate checksumBehavior key from yarn file (#25730)
  [FIX] Custom emoji reaction size (#25393)
  Chore: Test for department screen (#25696)
  Chore: Taking out Blaze from routes with `MainLayout`  (#25697)
  ...
@murtaza98 murtaza98 mentioned this pull request Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants