Skip to content
This repository has been archived by the owner on Oct 28, 2019. It is now read-only.

267 use async await #275

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

Conversation

Dufgui
Copy link
Contributor

@Dufgui Dufgui commented Oct 27, 2019

Please make sure the below checklist is followed for Pull Requests.

  • Travis tests are green
  • Tests are added where necessary
  • Documentation is added/updated where necessary
  • Coding Rules & Commit Guidelines as per our CONTRIBUTING.md document are followed

@Dufgui
Copy link
Contributor Author

Dufgui commented Oct 27, 2019

I know it's deprecated. But someone like use, us it. So you can merge it to simplify the usage.

@@ -33,7 +33,7 @@ module.exports = {
* @param root {Object} the document's root.
* @return {string} the editor's name.
*/
function detect(root) {
async function detect(root) {
Copy link
Member

Choose a reason for hiding this comment

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

perhaps it would be better to refactor this to remove the path where we ask the user to choose the editor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will do it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get back to this, I don't see what you want. I think we must split in different Merge request

"inquirer": "6.3.1",
"jhipster-core": "3.6.14",
"jhipster-core": "3.6.6",
Copy link
Member

Choose a reason for hiding this comment

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

interesting, why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because even if it’s a minor version api change. The values function. Has been removed. Other way. I copy it and use last dep

Copy link
Member

Choose a reason for hiding this comment

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

I don't get it, what has changed exactly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 named "values" has been removed

Copy link
Member

Choose a reason for hiding this comment

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

Ah, just replace ObjectUtils.values by Object.values

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's not ok what i have done?

@Dufgui Dufgui force-pushed the 267_use_async_await branch 2 times, most recently from 8ca089f to 6e0ffb8 Compare October 28, 2019 08:36
@deepu105
Copy link
Member

I don't see any point in merging PRs here, the project is deprecated for a reason, so that we don't spend any time on on it. @Dufgui why don't you fork it make the changes you want and use it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants