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

Convert function declaration to arrow function #114

Merged
merged 4 commits into from
Aug 13, 2020

Conversation

OliverJAsh
Copy link
Contributor

@OliverJAsh OliverJAsh commented Jun 29, 2020

Re. #113

@nicoespeon The tests pass but the code action does not appear. What am I missing? Figured it out, the import in extension.ts was missing. Perhaps this should be part of the code generation?

P.S. your contributing guide is fantastic.

@nicoespeon
Copy link
Owner

Figured it out, the import in extension.ts was missing. Perhaps this should be part of the code generation?

Good point, that's something missing.

Actually, I didn't put it in the code generation because I order the imports in extension.ts alphabetically. So I do that by hand.

There's also the package.json that should be configured. For this, at some point, I think I'll make it generate automatically so there's less manual work and potential errors. Maybe we could even make the extension.ts smarter so we don't really have to touch it again and it's all about configuration in each individual refactoring 🌈 ☀️

I think the simplest way to solve this today is to mention it in the contributing guide, to explain the need for import + order alphabetically. That will at least help future contributors waste less time.

@OliverJAsh OliverJAsh marked this pull request as ready for review June 30, 2020 12:15
@OliverJAsh
Copy link
Contributor Author

I think the simplest way to solve this today is to mention it in the contributing guide, to explain the need for import + order alphabetically. That will at least help future contributors waste less time.

Done in #116

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

Successfully merging this pull request may close these issues.

2 participants