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 svelte example for xlsx-import #157

Merged
merged 3 commits into from
Apr 6, 2021

Conversation

Metastasis
Copy link
Contributor

@Metastasis Metastasis commented Feb 6, 2021

Description

Resolves Svelte example from #6


EDIT @Siemienik :

@codecov
Copy link

codecov bot commented Feb 6, 2021

Codecov Report

Merging #157 (c12a039) into master (e29c158) will not change coverage.
The diff coverage is n/a.

❗ Current head c12a039 differs from pull request most recent head f94cc83. Consider uploading reports for the commit f94cc83 to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##            master      #157   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           54        54           
  Lines         1061      1061           
  Branches       153       153           
=========================================
  Hits          1061      1061           
Flag Coverage Δ
xlsx-import 100.00% <ø> (ø)
xlsx-renderer 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e29c158...f94cc83. Read the comment docs.

@Siemienik Siemienik self-requested a review February 6, 2021 16:36
@Siemienik
Copy link
Owner

Thank you @Metastasis 🥇
I remember about this PR and I going to review it soon 😃

@Siemienik
Copy link
Owner

I still remember and partially reviewed :)

Copy link
Owner

@Siemienik Siemienik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally, I reviewed it ;) Thank you for your contribution. Looks great to me 🥇

I tested it locally, and I run tests, all works.

I'm a little bit confused about the result of npm run validate, which return a set of errors with types that seem unable to work. (As You linked pyoner/svelte-typescript#23 ). I have to consider what should we do with that fact. I guess may we use just ES without TS? (TypeScript without typing imports seem to be fiction ;) )

WDYT?

samples/xlsx-import+svelte/src/App.svelte Outdated Show resolved Hide resolved
@Metastasis Metastasis force-pushed the feature/6-svelte branch 2 times, most recently from a4a8f23 to 1ef05fd Compare March 16, 2021 22:03
@Siemienik Siemienik self-requested a review March 22, 2021 22:08
samples/xlsx-import+svelte/README.md Outdated Show resolved Hide resolved
samples/xlsx-import+svelte/README.md Show resolved Hide resolved
@Siemienik
Copy link
Owner

Siemienik commented Mar 28, 2021

@Metastasis Great job :-) Thank you, Types are working now;)
I had only two minor comments, and I'm going to merge soon after README refilled 😄

@Siemienik Siemienik merged commit f6f0a7d into Siemienik:master Apr 6, 2021
@Siemienik
Copy link
Owner

@Metastasis Thank you very much for that great contribution 👍

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