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.

Sign up for GitHub

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

Jump to bottom

Refactoring and update of dependencied #55

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Refactoring and update of dependencied #55

wants to merge 6 commits into from

Commits on Mar 1, 2019

  1. Var replaced by let and consts in all files 

    Because of google recommendation from:

> https://google.github.io/styleguide/jsguide.html#features-use-const-and-let

Further discussion:

> https://stackoverflow.com/questions/41282992/google-js-coding-conventions-the-var-keyword-must-not-be-used
    @gustawdaniel
    gustawdaniel committed Mar 1, 2019
    Configuration menu
    Copy the full SHA
    1916651 View commit details
    Browse the repository at this point in the history

  2. Utit.inherits removed because of is discouraged 

    In official node.js docs there is written:

Usage of util.inherits() is discouraged. Please use the ES6 class and extends keywords to get language level inheritance support. Also note that the two styles are semantically incompatible.

In this commit this problem is solved. Class syntax is used and replacing prototypes.

Sources for further reading:

> https://nodejs.org/api/util.html#util_util_inherits_constructor_superconstructor
> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes
> nodejs/node#4179
> https://medium.com/@parsyval/javascript-prototype-vs-class-a7015d5473b
    @gustawdaniel
    gustawdaniel committed Mar 1, 2019
    Configuration menu
    Copy the full SHA
    a338901 View commit details
    Browse the repository at this point in the history

  3. Defined sets of events of imap and notifier 

    For the purpose of clarify what exactly happens in code, which events are possible, which impossible, and what it does mean there was created list of events.

It is recommended to use these constants instead of names of event in stings because it allow to differentiated between events from different sets.

For example event 'error' or 'end' can be understood differently in context of imap or notifier.
    @gustawdaniel
    gustawdaniel committed Mar 1, 2019
    Configuration menu
    Copy the full SHA
    69057f0 View commit details
    Browse the repository at this point in the history

  4. Imap and Notifier events replaced by constants 

    Instead of simple string in code constants wit events names are applied. It allow to differentiated between events of different objects that can have the same name. It allows to manage names of events independent to logic of code.

Sources:
Pros of this approach (in php but it desn't matter)
> https://pehapkari.cz/blog/2017/07/12/the-bulletproof-event-naming-for-symfony-event-dispatcher/
Example of merge in big js project where constants were used as names of events
> mochajs/mocha#3655

Opposed opinion (why constant is bad idea):
> https://stackoverflow.com/questions/42713927/why-arent-constants-used-for-events-in-node-js

So I invite to the discussion if you think that these changes are bad.

Why `require('events')` instead `require('events').EventEmitter`? Both forms works but proposed in this commit is now recommended and agreed in current documentation. 

Further reading:
Edit of first answer there
> https://stackoverflow.com/questions/21557369/javascript-requireevents-eventemitter
Official docs:
> https://nodejs.org/api/events.html
Code with backward compatibility
> https://github.com/nodejs/node-v0.x-archive/blob/master/lib/events.js
    @gustawdaniel
    gustawdaniel committed Mar 1, 2019
    Configuration menu
    Copy the full SHA
    683df88 View commit details
    Browse the repository at this point in the history

  5. Events for Fetch and Message saved as const 

    Changes similar to these for Imap and Notifier events.
    @gustawdaniel
    gustawdaniel committed Mar 1, 2019
    Configuration menu
    Copy the full SHA
    3a6e1a0 View commit details
    Browse the repository at this point in the history

  6. Packages async, debug and imap updated to latest version 

    Not all packages are updated. There is problem with mialparser. I believe it will be solved in further commits. Now there are added comments and problem is highlighted.
    @gustawdaniel
    gustawdaniel committed Mar 1, 2019
    Configuration menu
    Copy the full SHA
    628ad1c View commit details
    Browse the repository at this point in the history