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

Feature/lint files #89

Closed
wants to merge 11 commits into from
Closed

Feature/lint files #89

wants to merge 11 commits into from

Conversation

tzmanics
Copy link
Contributor

Linted lib/commands directory files.

modulus.io.print('Add-Ons provisioned for ' + projectName.verbose);
// ----------------------------------------------------------------------------
addOn.printList = function (projectName, addons) {
var addonConfig,configItem, i;
Copy link
Contributor

Choose a reason for hiding this comment

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

You need a space between the first two items. And can you put i first? At first I didn't see it...would be more obvious at the front.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

}
], function(err, results) {
modulus.io.success('Feedback sent. Thank you for the message.');
], function (results) { // ignores 'error'
Copy link
Contributor

Choose a reason for hiding this comment

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

err is still the first param, even if you ignore it. You probably meant to remove all params since it does not appear to use any of them

@tzmanics tzmanics force-pushed the feature/lint-files branch 3 times, most recently from 150223e to c8dc663 Compare February 25, 2016 05:21
@tzmanics
Copy link
Contributor Author

updated @modulus/standard and tested everything, looks good.
was everyone able to look through lib/project.js?

@jackboberg
Copy link
Contributor

@tzmanics I think this branch looks good, ready to review sub-branches as you complete them

@jackboberg
Copy link
Contributor

@tzmanics I'm afraid all this work might be a lost cause 😢 :

  1. we no longer use @modulus/standard internally, finally gave in and just use standard now
  2. it is likely more work than is worth it just to get this back to not having conflicts

How do you feel about closing it and I will give it another shot

@jackboberg
Copy link
Contributor

created #163 which converts the project to standard with very little other refactoring

@jackboberg
Copy link
Contributor

Replaced by #163

Thanks @tzmanics for the effort!!

@jackboberg jackboberg closed this Feb 10, 2017
@jackboberg jackboberg deleted the feature/lint-files branch February 10, 2017 17:31
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