Closed ofu997 closed 4 months ago
Hey @ofu997! Thanks for your contribution to the project.
I have a general sense of what the changes are, but would like to have the PR description updated to detail what you've done/changed for this PR. It'll help let other users get a feel for what this PR is for.
Also, keep note of unused imports and other errors provided by the linter and update the codebase accordingly (see lint check on GitHub: https://github.com/codeforpdx/PASS/actions/runs/6794649087/job/18471386525?pr=505).
Also, there seems to be linting errors. Do you run the lint or prettier in your branch? It doesn't seem like prettier is being picked up in your PR.
I ran npm run prettier:test
without error.
On Wed, Nov 15, 2023 at 12:17 PM Ka Hung Lee @.***> wrote:
Also, there seems to be linting errors. Do you run the lint or prettier in your branch? It doesn't seem like prettier is being picked up in your PR.
— Reply to this email directly, view it on GitHub https://github.com/codeforpdx/PASS/pull/505#issuecomment-1813199709, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFPDS72QAMM3HN3U373PL5TYEUPMXAVCNFSM6AAAAAA7CLCSMCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJTGE4TSNZQHE . You are receiving this because you were mentioned.Message ID: @.***>
I ran
npm run prettier:test
without error.
We don't have a script for prettier:test
, see screenshot.
npm run prettier:run
is the script for running Prettier to update formatting. npm run prettier:check
is the script for running Prettier to check formatting.
Weird
Weird
You need to run npm run prettier:run
. For npm run prettier:check
it's used by the CI to double check if formatting was missing.
Note the error found in the check:
Hey @ofu997 ! Wanted to check on the progress of this PR.
Believe we could start getting the discussions for where to place these new fields with the UX team once we got this merged and start an issue opened for implementation.
I'll make it https://schema.org/roleName
I'll make it https://schema.org/roleName
That looks like a good solution to me
Hey @ofu997! Was wondering if this is still being worked on. If you are, let us know. Thanks!
Hey @ofu997! Was wondering if this is still being worked on. If you are, let us know. Thanks!
Hi Kahung, It looks like I said "I'll make it https://schema.org/roleName" but didn't make a commit. I'm at work RN, so If it seems pretty much ready you can finish this up.
This PR:
Resolves #351
Screenshots (if applicable):
Additional Context (optional):
Add relation and relationship status to contact. We are currently (Nov 14) not sure whether we will set this in the add contact modal or from the table in the contact list.
Currently this creates two new constants files, and changes the useContactsList hook.
Future Steps/PRs Needed to Finish This Work (optional):
Add any other steps/PRs that may be needed to continue this work if this PR is just a step in the right direction.
Issues needing discussion/feedback (optional):
1. I'm unable to do the pod field which is the second field in the screenshot.
add this field: pod: <podUrl>, // optional: the person's preferred pod url. If not present, we fetch the first pod url on the profile document. If that's not present, we derive it from the webid.
When I tried to do it in previous commits there would be Promise or auth errors. This needs to be done later as a separate issue.