Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Conversation

@bmperrea
Copy link
Contributor

Description of the Change

Change lib/project/results-model.coffee to lib/project/results-model.js using decaffeinate and some human linting.

Alternate Designs

We could leave it as .coffee, but this comment suggests things are moving to ES6, and I would work on this file if it was in Javascript.

I could save this translation as part of a feature PR, but I thought it was useful to make translation a PR of it's own so that the changes on top of that will be easy to track.

Benefits

People don't have to figure out coffeescript to contribute.

Possible Drawbacks

You can't use coffeescript for this file anymore

Applicable Issues

I might add function-replace for project as in issue #417 if I can use es6 Javascript.

@jasonrudolph
Copy link
Contributor

I could save this translation as part of a feature PR, but I thought it was useful to make translation a PR of it's own so that the changes on top of that will be easy to track.

It's especially useful and appreciated to have this change as its own PR. 💟

Would you mind converting the code to standard style. I recommend running:

npm install -g standard
standard --fix lib/project/results-model.js

In addition to adjusting the code in place, the output may include some additional suggested edits. If so, can you please address those as well?

Once that's done, please let us know and we can review this PR and work toward getting it merged.

the only command line output that I left is "atom undefined"
@bmperrea
Copy link
Contributor Author

bmperrea commented Sep 2, 2017

@jasonrudolph - thanks for the suggestion - done with that.

@Ben3eeE Ben3eeE requested a review from jasonrudolph September 3, 2017 18:36
Copy link
Contributor

@jasonrudolph jasonrudolph left a comment

Choose a reason for hiding this comment

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

@bmperrea: Thanks for the updates. ⚡

I pushed up a few tweaks (b076686...478d93d). Once the build finishes running, I'll merge this PR.

@bmperrea
Copy link
Contributor Author

bmperrea commented Sep 5, 2017

👍

@jasonrudolph jasonrudolph merged commit 293aae4 into atom:master Sep 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants