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

Revisions to fileUpload component #79

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

alderete-sfdc
Copy link
Contributor

Edit and add comments. Refactoring. Minor functional changes. New bugs (probably).

Edit and add comments. Refactoring. Minor functional changes. New bugs (probably).
@alderete-sfdc alderete-sfdc self-assigned this Nov 4, 2023
@alderete-sfdc alderete-sfdc requested a review from a team as a code owner November 4, 2023 00:04
@alderete-sfdc alderete-sfdc removed their assignment Nov 4, 2023
@alderete-sfdc
Copy link
Contributor Author

@Taost I'm #finally back from PTO, and would love to push this to completion this week. I've moved the logging line you commented on, and added a few additional log and comment lines. Otherwise unchanged.

Any idea why I'm failing the prettier and run-tests checks? Prettier just "exits with error code 1", and the test failure looks like it's elsewhere in the repo. I'm not sure how to resolve either of those...

@Taost
Copy link
Contributor

Taost commented Nov 21, 2023

@Taost I'm #finally back from PTO, and would love to push this to completion this week. I've moved the logging line you commented on, and added a few additional log and comment lines. Otherwise unchanged.

Any idea why I'm failing the prettier and run-tests checks? Prettier just "exits with error code 1", and the test failure looks like it's elsewhere in the repo. I'm not sure how to resolve either of those...

@alderete-sfdc Thanks for addressing the comment.

To fix the test, you can update the test file here. The label has been changed from "Pick file to upload" to "Select file to upload".

To fix the prettier issue, you can run npm run prettier command in the Terminal window.

This is an attempt to resolve the CI errors in GitHub.
@alderete-sfdc
Copy link
Contributor Author

@Taost thanks for the tips! I was able to resolve the test failure (doh! who looks at tests! ;- ), and running prettier does make a change to one of the relevant-to-this-PR component files. I've committed those changes.

However, I am still getting prettier errors, on other components, and checking in the prettier changes for other components didn't resolve that. It also ran into conflicts with two of those components. I reverted that commit.

I'm reluctant to go mucking around in other people's code, because I'm not very involved with this repo (yet?), and don't know what considerations there are.

Is there any way to turn this over to a real engineer who knows this repo well enough to resolve the issues safely?

@dbreese
Copy link
Contributor

dbreese commented Nov 27, 2023

@alderete-sfdc I just pulled your code down and ran npm run prettier and it just updates fileUpload.js with 2 minor updates. What errors are you seeing while running prettier?

Here's the file difference as shown in Fork for me showing the two added commas:

image

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.

4 participants