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

HPC-9142: Remove entitiesAssociation model and its usages #147

Merged
merged 2 commits into from
Jun 6, 2024

Conversation

enxtur
Copy link
Contributor

@enxtur enxtur commented Apr 19, 2024

PR-144 needs to be merged first

@enxtur enxtur added the pending prior merge Another Pull Request needs to be merged before this one label Apr 19, 2024
@enxtur enxtur self-assigned this Apr 19, 2024
@enxtur enxtur marked this pull request as ready for review April 19, 2024 19:18
@enxtur enxtur requested a review from a team as a code owner April 19, 2024 19:18
@enxtur enxtur assigned Pl217 and unassigned enxtur Apr 19, 2024
@enxtur enxtur added the ready for review All comments have been addressed, and the Pull Request is ready for review label Apr 19, 2024
@enxtur enxtur changed the title HPC-9142: Remove entitiesAssociation usages HPC-9142: [2] Remove entitiesAssociation usages Apr 22, 2024
Copy link
Contributor

@Pl217 Pl217 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of basing this branch off develop, you should have used HPC-9142 as a base branch. The way you did it, commit c8b3b86 is duplicated. This work will need a rebase after #144 is merged. If you based this branch against HPC-9142, after merging the other PR, this one would auto-change its base to be develop.

But, regardless of that, I need to request changes here, because you didn't remove entitiesAssociation model, which I asked in this review comment.

@Pl217 Pl217 assigned enxtur and unassigned Pl217 Apr 22, 2024
@Pl217 Pl217 added needs minor changes There are review or issue comments to address and removed ready for review All comments have been addressed, and the Pull Request is ready for review labels Apr 22, 2024
@enxtur enxtur changed the base branch from develop to HPC-9142 April 22, 2024 15:10
@enxtur enxtur assigned Pl217 and unassigned enxtur Apr 22, 2024
@enxtur enxtur added ready for review All comments have been addressed, and the Pull Request is ready for review and removed needs minor changes There are review or issue comments to address labels Apr 22, 2024
@enxtur enxtur requested a review from Pl217 April 22, 2024 15:14
Base automatically changed from HPC-9142 to develop April 23, 2024 15:29
@Pl217 Pl217 changed the title HPC-9142: [2] Remove entitiesAssociation usages HPC-9142: Remove entitiesAssociation model and its usages May 10, 2024
src/lib/data/planEntities.ts Outdated Show resolved Hide resolved
@Pl217 Pl217 assigned enxtur and unassigned Pl217 May 10, 2024
@Pl217 Pl217 added needs minor changes There are review or issue comments to address and removed ready for review All comments have been addressed, and the Pull Request is ready for review labels May 10, 2024
@enxtur enxtur assigned Pl217 and unassigned enxtur May 17, 2024
@enxtur enxtur added ready for review All comments have been addressed, and the Pull Request is ready for review and removed needs minor changes There are review or issue comments to address labels May 17, 2024
@enxtur enxtur requested a review from Pl217 May 17, 2024 10:58
Copy link
Contributor

@Pl217 Pl217 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that removal of entitiesAssociation model should mean that we need a major version bump when we release next version of this package.

@Pl217 Pl217 assigned enxtur and unassigned Pl217 Jun 3, 2024
@Pl217 Pl217 removed the ready for review All comments have been addressed, and the Pull Request is ready for review label Jun 3, 2024
@enxtur enxtur merged commit 900b587 into develop Jun 6, 2024
2 checks passed
@enxtur enxtur deleted the HPC-9142-2 branch June 6, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending prior merge Another Pull Request needs to be merged before this one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants