-
-
Notifications
You must be signed in to change notification settings - Fork 207
fix: Select input name instead of file in ParseFile
#1012
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
fix: Select input name instead of file in ParseFile
#1012
Conversation
I will reformat the title to use the proper commit message syntax. |
Thanks for opening this pull request! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1012 +/- ##
==========================================
+ Coverage 43.37% 43.48% +0.11%
==========================================
Files 61 61
Lines 3463 3463
==========================================
+ Hits 1502 1506 +4
+ Misses 1961 1957 -4 ☔ View full report in Codecov by Sentry. |
ParseFile
The two failed items are not related to the codes of this PR. I will check these two items in another PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a test for this?
Added. |
It seems that codecov still reported the new code as not covered. Is there a test case missing? https://github.com/parse-community/Parse-SDK-Flutter/pull/1012/checks?check_run_id=31443404272 |
No, there is no problem. I have written a test for exactly the same line that had problem. Codecov seems to have not detected well ! |
Why are the comments removed? These comments are based on the project's test writing standard Please check these files. https://github.com/parse-community/Parse-SDK-Flutter/tree/master/packages/dart/test/src |
I don't think these comments add any value, nor are they currently consistently used, nor are they practical. If you look at you can see that they are all over the place, sometimes missing, sometimes they are mixed with other comments with seems confusing. It's obvious that an I'd remove them over time and avoid them for new tests. |
|
Alright, let's disregard the coverage warning for that line. |
What should we do about the failing beta in the CI? I think we've added it to keep the codebase compatible for the new release. Is it failing because of a change in this PR, or is that unrelated? |
The reason ci fails is a warning linter in the beta version of Dart and Flutter and is not related to this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, if you could add the version change then we go ahead and merge.
added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Pull Request
Issue
Select the file name from the input name instead of file
Closes: #1011
Approach
n/a
Tasks