Skip to content

Conversation

Ken473
Copy link
Collaborator

@Ken473 Ken473 commented Oct 18, 2021

This is the first pass of the refactoring for the Channels and the clients. This is for you to see and comment if you're happy with the approach we have taken. We are going to add more refactorings and apply those to the rest of the entities.

@michaelloosen
Copy link
Contributor

Please fix linting and look at failing tests

Copy link
Contributor

@MattyJ007 MattyJ007 left a comment

Choose a reason for hiding this comment

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

@Ken473
Please copy over only the necessary logic changes into a new branch and open a new PR.
The changes here are convoluted and hidden within the linting - this is not acceptable practice. If large scale linting is required, it should be done as a separate commit. This makes logic changes easier to follow commit by commit instead of having to trawl through the 1450 lines of code you touched.

The formatting of the files is now also inconsistent with the project standard. Please use the linting tool included in the node dependencies - the command was included in the package.json - npm run lint:fix

You have also included committed multiple console.log statements throughout your code - an accidental commit occasionally is human - this level is sloppy. Please check the work you are adding with a diff tool or use git add -p to add your work in stages to prevent adding unnecessary lines.

@MattyJ007
Copy link
Contributor

Also, please include a message body and the ticket in your commit as well - not just a heading.
The body should explain why you are making the change. Poor commit messages make maintaining a codebase difficult - especially in a large scale API change.

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