Skip to content

[FLOW-73] add Category#54

Merged
rmustafin merged 4 commits intomasterfrom
FLOW-73/categories
Mar 8, 2021
Merged

[FLOW-73] add Category#54
rmustafin merged 4 commits intomasterfrom
FLOW-73/categories

Conversation

@rmustafin
Copy link
Contributor

@rmustafin rmustafin commented Mar 7, 2021

Intent (What)

Add Category

https://developer.xero.com/documentation/practice-manager/categories

Motivation (Why)

We want XpmRuby::Category for our new cool features

Implementation (How)

Consequence

@rmustafin rmustafin marked this pull request as ready for review March 7, 2021 23:09
@rmustafin rmustafin requested a review from a team as a code owner March 7, 2021 23:09
@rmustafin rmustafin requested a review from rudylee March 8, 2021 01:01
module Category
extend self

class Error < Error; end
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
class Error < Error; end
class Error < StandardError; end

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually why do we need to define this here ? I noticed other modules also have this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I think it might be a severe case of 🍝

Copy link
Contributor

Choose a reason for hiding this comment

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

@abreckner might be able to answer. I'll say just delete it if we don't use it.

Copy link
Contributor

@rudylee rudylee left a comment

Choose a reason for hiding this comment

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

Looks good, just one question about the error class

@rmustafin rmustafin merged commit a1de80e into master Mar 8, 2021
@rmustafin rmustafin deleted the FLOW-73/categories branch March 8, 2021 04:07
lankz pushed a commit that referenced this pull request Jun 10, 2025
lankz pushed a commit that referenced this pull request Jun 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants