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

Adding fixes for issues raised by @jpmckinney #71

Merged
merged 2 commits into from
Feb 14, 2017

Conversation

aepton
Copy link
Contributor

@aepton aepton commented Feb 14, 2017

Closes #68

Value to set for the attribute in the Committee object.

description
String containing whatever associated text we got along with the status
Copy link
Member

Choose a reason for hiding this comment

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

Change 'status change' to 'attribute change'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -268,7 +276,7 @@ id
Open Civic Data-style id in the format ``ocd-campaignfinance-candidateorientation/{{uuid}}``

candidate
Candidate
OCD Person representing the candidate
Copy link
Member

Choose a reason for hiding this comment

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

Change to 'OCD Person indicating the candidate' to avoid any confusion in the meaning of 'representing', and for consistency with other definitions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -23,7 +23,8 @@ Filing
Any single document filed with a Regulator.

Jurisdiction
The region covered by an Office, or for which an Election is being held.
OCD Jurisdiction indicating the region covered by an Office, or for which an
Copy link
Member

Choose a reason for hiding this comment

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

Lowercase 'Office' as Office is not a class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Questions to answer
-------------------
* How to model Committee Status and Committee Purpose inside of OCD Organization
models?
Just add additional fields for those. Not sure why this was ever a question.
Copy link
Member

Choose a reason for hiding this comment

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

We can add a purpose field to Organization, like on W3C ORG, on which Organization is based. Open a new issue for that, and link from the answer to this question?

Copy link
Member

Choose a reason for hiding this comment

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

Similarly, W3C Registered Organization has a orgStatus field, which we can consider reusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

* How to handle different types of Committees beyond just imputing their types
based on (say) whether they are oriented toward more than one Candidate?
I don't think this is an issue.
Copy link
Member

Choose a reason for hiding this comment

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

Delete the question in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

* How to reconcile multiple reports that describe the same contribution,
expenditure or other event?
Same transactions should have same ids. Multiple filings can point to one
Copy link
Member

Choose a reason for hiding this comment

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

Can you capitalize ID here and elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants