-
Notifications
You must be signed in to change notification settings - Fork 466
github-pull-request-board decouple board from entity page #4710
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
Conversation
Changed Packages
|
71d600f
to
c82e584
Compare
c82e584
to
7040e10
Compare
494d047
to
56dfdd8
Compare
…laces Signed-off-by: Juan Pablo Garcia Ripa <[email protected]>
56dfdd8
to
52c0aa0
Compare
Signed-off-by: Juan Pablo Garcia Ripa <[email protected]>
This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution! |
Not stale |
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.
Thanks for the contribution @Sarabadu, left one comment but overall looks good to me 👍
workspaces/github-pull-requests-board/plugins/github-pull-requests-board/package.json
Outdated
Show resolved
Hide resolved
Signed-off-by: Juan Pablo Garcia Ripa <[email protected]> Signed-off-by: Juan Pablo Garcia Ripa <[email protected]>
repositories: teamData.repositories, | ||
teamMembers: teamData.teamMembers, | ||
teamMembersOrganization: getGithubOrganizationFromEntity(teamEntity), | ||
teamMembersOrganization: getGithubOrganizationFromEntity(ownerEntities[0]), |
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.
I'm not familiar with this plugin, does this mean it is always derived from the first entity? Perhaps that is intentional?
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.
thanks, yes is intended, that field is used on the current EntityPage, that has always only 1 entity.
in the future if multiple organization tracking is needed i think it would be better to make a bigger refactor to decouple it a bit more, but as it now I tried to no go full down in the rabbit hole
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.
LGTM, assuming review comments considered. Dropping an a approving so this isn’t blocked while I’m OOO.
@awanlin, if you have some time, we still need your approval after the change request |
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.
Let get this out
Hey, I just made a Pull Request!
Why
We would like to use the Pull request board inside of the
Manage
plugin page and there is some in progress work to show the PR data inside the home page too. but ro now the plugin requires to be inside the EntityProvider context scope.What
On this PR we are decoupling the EntityProvider from the rest of the board query and views.
Changes:
useEntity
from theuseUserRepositoriesAndTeam
hook, and accepting one or more Entitiesdev
folder to have some better way to test it locallyThe existen componentes should not get affected by this change but we are exporting a new React component to be posible to use in other places
✔️ Checklist
Signed-off-by
line in the message. (more info)