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

Added options to disable generation of serializers, to/from json, and… #28

Closed
wants to merge 2 commits into from

Conversation

n3rdyme
Copy link

@n3rdyme n3rdyme commented Feb 15, 2020

  • Added options to disable generation of serializers, to/from json, and services.
  • Added header to disable es-lint and ts-check

@n3rdyme
Copy link
Author

n3rdyme commented Feb 15, 2020

@stephenh If you need a merged pull request for #28 and #31 just let me know.

src/main.ts Outdated Show resolved Hide resolved
src/plugin.ts Show resolved Hide resolved
@n3rdyme
Copy link
Author

n3rdyme commented Feb 16, 2020

@stephenh
I hope I addressed all the questions and asks above. As to the source comments for disabling linters, I feel pretty strongly that code generators should provide this. If you are against it being on by default, perhaps we can make this an option instead?

@stephenh
Copy link
Owner

I'll locally merged this PR and pushed it to master (there was a minor conflict from your other PR).

I also made the longToNumber and DeepPartial output conditional, so removed ts-nocheck but kept always outputting eslint-disable.

If you have more examples of ts-nocheck-requiring behavior, I'm happy to try and fix those too. I'm slightly (slightly :-)) open to a outputTsNoCheck config flag.

@stephenh stephenh closed this Feb 16, 2020
@stephenh
Copy link
Owner

Can you check how master works for you with your two PRs merged and then I'll make a release?

Thanks again!

@n3rdyme
Copy link
Author

n3rdyme commented Feb 17, 2020

@stephenh I tested from latest master:

commit a7a20484bdd8dc96d402090060e9696fac89ebcb (HEAD -> master)
Author: Stephen Haberman <[email protected]>
Date:   Sun Feb 16 15:54:58 2020 -0600

    Link.

Everything is working for me. Thanks for the merge!

BTW... if anyone does need @ts-nocheck, they can do this at the top of the offending proto files:

//@ts-nocheck
syntax = "proto3";

...

Which then outputs in the source as needed:

/* eslint-disable */
// @ts-nocheck
//
import { Empty } from '../../google/protobuf/empty';
...

So, I guess that works ;)

@stephenh
Copy link
Owner

So, I guess that works ;)

Lol, that's hilarious. :-)

Great that it works for you. I'd gone ahead and released master (for a few non-doc commits before that) as v1.11.0.

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