-
Notifications
You must be signed in to change notification settings - Fork 9
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
Migrate multiple vuex stores to pinia #256
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Gavin Jaeger-Freeborn <[email protected]>
Signed-off-by: Gavin Jaeger-Freeborn <[email protected]>
Signed-off-by: Gavin Jaeger-Freeborn <[email protected]>
Signed-off-by: Gavin Jaeger-Freeborn <[email protected]>
Signed-off-by: Gavin Jaeger-Freeborn <[email protected]>
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.
Changes are looking good overall, looking forward to the end result 🙂
src/store/modules/entity.ts
Outdated
console.log("fetching relationships"); | ||
const res = await relationshipService.getRelatedTo(topic_id); | ||
console.log(res); |
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.
Please don;t leave debug logs in the code. You may also want to name the variable something more explicit, rather than just res
as it makes understanding the code easier.
return ( | ||
(moment(item1.date_effective).isBefore(item2.date_effective) ? -1 : 1) * | ||
this.credentialTimeOrder | ||
const ret = state?.type + "." + state?.value; |
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.
Can this variable be named more explicitly?
Signed-off-by: Gavin Jaeger-Freeborn <[email protected]>
This is This is the first half of the migration from vuex to pinia stores #242