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.

Sign up for GitHub

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

Jump to bottom

feat: ban typescript Omit type [no issue] #218

Merged
merged 1 commit into from
Jul 9, 2019
Merged

feat: ban typescript Omit type [no issue] #218

merged 1 commit into from
Jul 9, 2019

Conversation

christophehurpeau
Copy link
Contributor

@christophehurpeau christophehurpeau commented Jul 9, 2019 •
edited
Loading

Context

La version d'Omit inclue dans typescript n'est pas stricte. Voir issue microsoft/TypeScript#30825

Solution

Eslint permet de bannir ce type: microsoft/TypeScript#30825 (comment)

Options:

  • This PR is a feature branch
  • Auto merge with [skip ci]
  • Auto merge when this PR is ready and has no failed statuses. (Also has a queue per repo to prevent multiple useless "Update branch" triggers)
  • Automatic branch delete after this PR is merged

@christophehurpeau christophehurpeau changed the title feat: ban omit type feat: ban typescript Omit type Jul 9, 2019
@christophehurpeau christophehurpeau changed the title feat: ban typescript Omit type feat: ban typescript Omit type [no issue] Jul 9, 2019
@LentnerStefan
Copy link
Contributor

LentnerStefan commented Jul 9, 2019 •
edited
Loading

@christophehurpeau Dans nos projets, il vaut mieux qu'on copie/colle le except de type-fest, ou qu'on ajoute la lib dans nos deps ?

@CorentinAndre
Copy link
Contributor

CorentinAndre commented Jul 9, 2019 •
edited
Loading

@LentnerStefan, @christophehurpeau J'ai regardé notre usage dans les différents projets TS de Omit:

  • components: pas utilisé pour le moment
  • learner-app: défini une fois comme dans type-fest
  • sherwood-react: un Omit exporté défini comme dans type-fest
  • kitt: Omit par défaut de typescript mais aussi Except

Je pense qu'on peut ajouter assez sûrement type-fest, et remplacer nos Omit par Except..

@christophehurpeau
Copy link
Contributor Author

christophehurpeau commented Jul 9, 2019 •
edited
Loading

@LentnerStefan qu'on ajoute la lib type-fest
@CorentinAndre c'est utilisé plusieurs fois dans la branche typescript de kitt

@CorentinAndre
Copy link
Contributor

@christophehurpeau https://github.com/ornikar/kitt/blob/508ecf35b3764ae7fedde04c901928d913f6d16b/src/components/InputAddress/index.tsx#L42, c'est voulu du coup le Omit ?

@reviewflow reviewflow bot added the 🔜 automerge Synced by reviewflow for merge/automerge label Jul 9, 2019
@reviewflow reviewflow bot merged commit 1596967 into master Jul 9, 2019
@reviewflow reviewflow bot deleted the feat/ban-omit-type branch July 9, 2019 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CorentinAndre CorentinAndre CorentinAndre approved these changes

Assignees

@christophehurpeau christophehurpeau

Labels
🔜 automerge Synced by reviewflow for merge/automerge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@christophehurpeau @LentnerStefan @CorentinAndre