add excel file support #22
Conversation
|
@Jitensid don't we have to link the PR to the original issue#5. Also, everything is running correctly in my system. can you help me understand the errors shown here. |
|
I checked your package.json file. Could you add the dependency like others ? |
|
Doesn't package.json update automatically when we run npm install? |
It does but why it is in a different format? |
What do you mean by different format? Are you not able to run it in your system? |
|
Check review comments |
|
In CI/test of windows and macOS, it says that lockfile version 1 is required but in the main branch also it says lockfile version 2 in package-lock.json. |
|
Just remove the package.lock json file and try again . Ideally it should not be pushed to repo. It should work after that. |
So should I add it to gitignore and then push again? |
|
@Jitensid It shows that the split method will have undefined, but it can never be since I am uploading a file. can you bypass the test and run it on your device once, all the functionalities are running correctly in my system. |
|
@Jitensid Why does it always wait for approval to test after the initial PR approval? |
|
@Jitensid there's some issue in the UploadButton.test.js as shown in the log, but I didn't even work on it. yeah I will look into this. |
|
@Jitensid I think so, the issue is due to the test being written for json file format and not compatible with other formats. |
|
@all4one-max I am running the code into my system |
|
I will need some time to figure out what exactly is the issue |
|
@Jitensid I want to start working on the other issue while you figure out the problem we are facing here. But I was wondering if I make a separate branch and start working on it, won't it cause an issue when this PR gets merged? Like I will start working on the branch from the code i.e in the main branch. nvm, I can't start working on it until this gets merged since both issues are to be handled in the upload Button file itself. Hoping for a quick resolution! |
|
|
The other issue I raised, upload tab files options.
…On Wed, 19 Oct, 2022, 9:50 am Jiten Sidhpura, ***@***.***> wrote:
@Jitensid <https://github.com/Jitensid> I want to start working on the
other issue while you figure out the problem we are facing here. But I was
wondering if I make a separate branch and start working on it, won't it
cause an issue when this PR gets merged? Like I will start working on the
branch from the code i.e in the main branch.
nvm, I can't start working on it until this gets merged since both issues
are to be handled in the upload Button file itself.
Hoping for a quick resolution!
In which you are planning to work?
—
Reply to this email directly, view it on GitHub
<#22 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOLKTGO7FTC4PGD75QV7QUDWD5ZJJANCNFSM6AAAAAARIFX3HE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Codecov Report
@@ Coverage Diff @@
## main #22 +/- ##
==========================================
- Coverage 85.61% 81.76% -3.86%
==========================================
Files 8 8
Lines 139 159 +20
Branches 15 23 +8
==========================================
+ Hits 119 130 +11
- Misses 19 29 +10
+ Partials 1 0 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@all4one-max test is passing now. |
|
@Jitensid how do I handle the codecov thing? wait! since you have made commits in the branch I will have to pull first right? |
|
Yeah so the issue is code coverage has dropped. |
hey will it help if I try to reduce code at some place (like write a shorter version). |
|
please revert your latest commit |
|
okay! |
|
I shall write the test for excel file upload in some time. |
|
https://app.codecov.io/gh/Jitensid/TabPort/pull/22 This will show where your code is not covered by test cases |
|
I see, but I don't know how to write test cases, I can try learning how but it might take a lot of time. |
No problem I shall write one for this case in some time. |
|
@all4one-max thanks for your contribution and I loved your dedication ! |
The following features have been added as per issue #5:
I have primarily used sheetJS package for implementing the features, so make sure to install all the dependencies.
I hope these features are as desired.