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

8438 cooperative schema update #65

Merged
merged 7 commits into from
Aug 13, 2021

Conversation

vysakh-menon-aot
Copy link
Collaborator

Issue #: /bcgov/entity#8438

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the business-schemas license (Apache 2.0).

@vysakh-menon-aot vysakh-menon-aot deleted the feature/8438 branch August 12, 2021 11:03
Comment on lines 63 to 65
"cooperativeAssociationType": {
"type": "string",
"title": "The association type for cooperative filing"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This could be an enum. We can do that once values are available.

Copy link
Collaborator

@thorwolpert thorwolpert left a comment

Choose a reason for hiding this comment

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

A few changes

@@ -92,4 +94,6 @@
'TRANSITION',
'TRANSITION_FILING_TEMPLATE',
'UNMANAGED',
'COOP_INCORPORATION',
Copy link
Collaborator

Choose a reason for hiding this comment

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

please keep in alphabetical order

@@ -1612,6 +1693,24 @@
}
}

COOP_INCORPORATION_FILING_TEMPLATE = {
'filing': {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the FILING_HEADER, easier to keep current

'email': '[email protected]',
'phone': '123-456-7890'
},
'cooperative': {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we can start making the examples fit the same as the schema definition?
a value for _ cooperative_ that is then added to the base incorporation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you suggest to include cooperative in incorporation (example) itself?

Copy link
Collaborator

Choose a reason for hiding this comment

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

in the default incorporation (core) no.
I think we should split these out so that we end up with core, and then IA-, ben, ltd, coop, etc.

@@ -51,8 +49,40 @@
},
"nameTranslations": {
"$ref": "https://bcrs.gov.bc.ca/.well_known/schemas/name_translations"
},
"cooperative": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

change this so it's its own section like nameRequest
and then keep it in alpha order

@@ -22,4 +22,4 @@
Development release segment: .devN
"""

__version__ = '2.13.1' # pylint: disable=invalid-name
__version__ = '2.13.2' # pylint: disable=invalid-name
Copy link
Collaborator

Choose a reason for hiding this comment

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

i mucked the numbers, will need to be 2.14.0

Copy link
Collaborator

@thorwolpert thorwolpert 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.
it should be good to go once the example data is updated

@thorwolpert
Copy link
Collaborator

I'm going to merge as I need to add in restoration and I don't want you to have to rebase on that for a simple change.

@thorwolpert thorwolpert merged commit 6105426 into bcgov:master Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants