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

The current master is failing in CI #39

Closed
limonte opened this issue Jun 10, 2018 · 5 comments
Closed

The current master is failing in CI #39

limonte opened this issue Jun 10, 2018 · 5 comments

Comments

@limonte
Copy link
Member

limonte commented Jun 10, 2018

https://travis-ci.org/sweetalert2/sweetalert2-react-content/builds/390371045

./node_modules/.bin/tsc src/sweetalert2-react-content.d.ts --lib dom,es6 

src/sweetalert2-react-content.d.ts:56:37 - error TS2344: Type 'keyof T' does not satisfy the constraint 'string'.
  Type 'string | number | symbol' is not assignable to type 'string'.
    Type 'number' is not assignable to type 'string'.

56 type Overwrite<T, U> = Pick<T, Diff<keyof T, keyof U>> & U;

@toverux would you mind to take a look on this issue?

@limonte
Copy link
Member Author

limonte commented Jun 17, 2018

@zenflow @toverux are you planning to maintain this project? If so, this issue should be fixed asap because:

  • greenkeeper is useless - it creates a lot of falsy issues with warnings
  • travis is useless - every build will be red, which means every contribution will result into failure which means poor contributor experience.

Let's stay the outstanding example of how OSS should be made and not join the vast majority of repositories which were published and then abandoned (Good Luck With That Public License).

@zenflow
Copy link
Member

zenflow commented Jun 17, 2018

@limonte Yes, I do plan to maintain this project.

Excuse for being absent: My laptop died while I was travelling in South America. It's not an ideal place to buy a new laptop because of duties on imports, and also I've been doing travel things. To make matters worse I was robbed in Chile and lost my credit and debit card (making it difficult to make a large purchase).

But as of yesterday, I'm in Buenos Aires in a new apartment with my girlfriend, for the next 2.5 months, and I have a new laptop (thanks! 😃😃😃).

So I aim to resolve this tomorrow.

@zenflow
Copy link
Member

zenflow commented Jun 17, 2018

Because of this issue and #33, I was thinking of just removing TS support for now.

I think the implementation is less than ideal right now anyways (just guessing), since it was done before the object-oriented refactor of SweetAlert2 and subsequently this library. I think it will have to be redone to be proper anyways. Sorry @toverux for not having the foresight about this.

If there's no objection to this then I'll make a PR tomorrow and leave it for a day or two to give @toverux a chance to intervene, though he seems to be busy.

@limonte
Copy link
Member Author

limonte commented Jun 18, 2018

I thank that TS is a must nowadays and its support is mandatory, let's not drop it.

This issue should be fixed with #43

In order to fix #33 we need to either learn TS or wait for @toverux :) TS is in my learning roadmap, I'll try to do it asap.

@zenflow
Copy link
Member

zenflow commented Jun 18, 2018

🎉 This issue has been resolved in version 1.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

No branches or pull requests

2 participants