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

Refactoring the createWebsocket Event #229

Merged
merged 2 commits into from Aug 1, 2019

Conversation

iFlameing
Copy link
Contributor

@datakurre I made a separate function for createwebsocketevent. We have to pass lots of arguments to the functions. I also refactor the updatePloneCollection function But somehow I am not able to update the plone collection because it is giving me an error

  TypeError: getNodes is not a function

Can you point out what I am missing? Or some better way to refactor the code

@iFlameing iFlameing requested a review from datakurre July 29, 2019 19:57
@iFlameing iFlameing added this to In Progress in GSOC 2019 Jul 29, 2019
@iFlameing iFlameing moved this from In Progress to Need help in GSOC 2019 Jul 29, 2019
@coveralls
Copy link

coveralls commented Jul 29, 2019

Pull Request Test Coverage Report for Build 767

  • 2 of 22 (9.09%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 37.752%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/gatsby-node.js 2 22 9.09%
Totals Coverage Status
Change from base Build 763: 0.2%
Covered Lines: 139
Relevant Lines: 351

💛 - Coveralls

@iFlameing
Copy link
Contributor Author

I am not feeling very good about this refactoring. It will reduce the noise but it is not looking good to me. we hardly saving any line :(

@datakurre
Copy link
Collaborator

@iFlameing Did you find and fix the issue you had with collection update? I guess you did, because this is now green.

For me this looks good. It does not matter that you don't save any lines. It's more important to "flatten" the code into smaller functions.

Once that's done, you could think about splitting these huge "utils.js" and "gatsby-node.js" files into smaller meaningful files (with matching tests files).

All this will make it easier to do typescript reafactoring later.

Single option to "reduce lines" would be to keep passing single "options" object instead of separate parameters, but I would like to do that only in typescript. Then typescript would project that no options are missing.

@datakurre datakurre merged commit aedb178 into collective:master Aug 1, 2019
GSOC 2019 automation moved this from Need help to Done Aug 1, 2019
@iFlameing
Copy link
Contributor Author

@datakurre Yes! After some experiminting, I am able to fix it :)

@iFlameing iFlameing deleted the createwebsocketevent branch August 1, 2019 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
GSOC 2019
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants