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

Remove unused imports #24

Closed

Conversation

matiasbastos
Copy link

@matiasbastos matiasbastos commented Mar 1, 2021

Removed unused imports to fix build errors (fix #25)

@matiasbastos
Copy link
Author

Hi @aayush-sib ,
Can you please give a look to my PR? I'm about to go live with a SiB integration and will be nice to be able to use latest lib ;)

@aayush-sib
Copy link
Contributor

Hi @matiasbastos
Sorry for the delay, I was waiting for the other PR to get updated. Can you please create a branch from this project only and push changes to that branch instead of pushing it to the forked project?
Thanks!

@yurist38
Copy link

Hi guys! Any updates on this? What stops this PR from merging? @aayush-sib I don't really understand why you suggest creating a branch in this repo instead of merging the forked one... What is the difference?

@aayush-sib
Copy link
Contributor

Hi @matiasbastos @yurist38
I have released the wrapper with some changes. Can you please check it at your end if it's working fine?

Thanks, @matiasbastos for raising the PR, we appreciate your efforts.

Regards
Sendinblue Team

@yurist38
Copy link

yurist38 commented Mar 30, 2021

@aayush-sib
Hi guys! It works for me how! 🎉 The issue has gone. Thank you very much for the fix! 👍

[UPDATE]:
Sorry, I was wrong. Actually, the issue did not disappear, the build still fails for me... :(

@aayush-sib
Copy link
Contributor

Hi @yurist38
Can you please tell me the error you are getting?

@matiasbastos
Copy link
Author

Sorry, I've been out for a while, this evening will retest. Thanks!

@yurist38
Copy link

yurist38 commented Apr 5, 2021

@aayush-sib
This is the error I see when I'm trying to build my Next.js project:

Screen Shot 2021-04-05 at 11 58 36

The error appears if I do import like this:

import { TransactionalEmailsApi } from 'sib-api-v3-typescript';

But when I require the library in this way:

const { TransactionalEmailsApi } = require('sib-api-v3-typescript');

then the error does not appear and the project builds successfully and works.

Right now I use require() but I'd like to use import instead...

@aayush-sib
Copy link
Contributor

Hi @yurist38
This error is something else than this. I am looking into it. This PR was mainly to remove unused imports.

@yurist38
Copy link

yurist38 commented Apr 6, 2021

@aayush-sib Oh, you're right. Sorry, I've mixed up this PR with another one #23 . This is one that should fix it.

@aayush-sib
Copy link
Contributor

Hi @yurist38
Can you please share the configuration which you are using for building the project?
Thanks!

@matiasbastos
Copy link
Author

Sorry, so what's missing here? Can this be merged?

@aayush-sib
Copy link
Contributor

Hi @matiasbastos

As you had already seen currently, we are using OpenAPI Generator to generate our typescript wrapper, we can't directly merge PR's since it's not recommended as we have to make all similar changes again when we will generate the wrapper. So, for this issue, I have introduced es lint in this wrapper which will remove all unused imports. I am still working on the other fix due to which the build is failing.

Also, we are in planning to introduce our own custom-made wrappers and in those wrappers, other developers can also contribute.

@matiasbastos
Copy link
Author

Hi @matiasbastos

As you had already seen currently, we are using OpenAPI Generator to generate our typescript wrapper, we can't directly merge PR's since it's not recommended as we have to make all similar changes again when we will generate the wrapper. So, for this issue, I have introduced es lint in this wrapper which will remove all unused imports. I am still working on the other fix due to which the build is failing.

Also, we are in planning to introduce our own custom-made wrappers and in those wrappers, other developers can also contribute.

That sounds great! So guess it's ok to close this PR as is going to be fixed in a future release?

@aayush-sib
Copy link
Contributor

So guess it's ok to close this PR

I haven't closed this PR because I was waiting for your response, whether this issue (unused imports issue) is fixed or not. If this issue is fixed you can close this PR 🙂 .

@aayush-sib aayush-sib closed this Sep 6, 2021
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.x version throw errors on build due to unused imports
3 participants