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

[K12] Add task to K12 yml to support New Picklists Value for Citizenship Status on Contact #296

Merged
merged 10 commits into from
Jan 29, 2021

Conversation

kdy618
Copy link
Contributor

@kdy618 kdy618 commented Dec 16, 2020

Critical Changes

Changes

Added a new task to K12 cumulus.yml file to reference an EDA task that adds new picklist value for hed__Citizenship_Status__c on Contact.

Issues Closed

Close #295

New Metadata

Unpackaged Metadata

Deleted Metadata

Testing Notes

@kdy618 kdy618 requested a review from a team as a code owner December 16, 2020 16:53
@kdy618 kdy618 changed the title [K12] Add flow to K12 yml to support New Picklists Value for Citizenship Status on Contact [K12] Add task to K12 yml to support New Picklists Value for Citizenship Status on Contact Dec 16, 2020
cumulusci.yml Outdated Show resolved Hide resolved
cumulusci.yml Outdated Show resolved Hide resolved
kdy618 and others added 2 commits December 16, 2020 12:12
Co-authored-by: Matthew Blanski <[email protected]>
Co-authored-by: Matthew Blanski <[email protected]>
MatthewBlanski
MatthewBlanski previously approved these changes Dec 16, 2020
Copy link
Contributor

@MatthewBlanski MatthewBlanski left a comment

Choose a reason for hiding this comment

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

Tentatively approved. This functioned when I changed over to the eda beta, which has the citizenship picklist task. We will need to wait until the release merge (which is appropriate) until we merge this in.

@spinkelman FYI
@jofsky FYI

@jofsky
Copy link
Contributor

jofsky commented Dec 16, 2020

@kdy618 I've discussed my idea of having an upstream EDA configuration task to be passed downstream to allow all the products to reduce code duplication. I'll approve this for now but lets make this part of our larger effort to reduce this code footprint.

cumulusci.yml Outdated Show resolved Hide resolved
jofsky
jofsky previously approved these changes Dec 16, 2020
Copy link
Contributor

@jofsky jofsky left a comment

Choose a reason for hiding this comment

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

@kdy618 I've discussed my idea of having an upstream EDA configuration task to be passed downstream to allow all the products to reduce code duplication. I'll approve this for now but lets make this part of our larger effort to reduce this code footprint.

@kdy618
Copy link
Contributor Author

kdy618 commented Jan 8, 2021

@MatthewBlanski @jofsky , I had to fix merge conflict. If you don't mind re-reviewing this PR. Thank you in advance.

@kdy618 kdy618 assigned MatthewBlanski and jofsky and unassigned MatthewBlanski Jan 8, 2021
MatthewBlanski
MatthewBlanski previously approved these changes Jan 11, 2021
@jofsky
Copy link
Contributor

jofsky commented Jan 19, 2021

@kdy618 what's the status of this?

@kdy618
Copy link
Contributor Author

kdy618 commented Jan 19, 2021

@jofsky, I initially was waiting for approval and based on Matt's initial response #296 (review) as to why this PR was not merged. I had to fix merge conflict when the cumulus file was updated and resubmitted for approval. It looks like @MatthewBlanski approved it and was waiting for your approval. However, it looks like it hit another merged conflict. Is there a better way for me to get this PR closed?

jofsky
jofsky previously approved these changes Jan 19, 2021
Copy link
Contributor

@jofsky jofsky left a comment

Choose a reason for hiding this comment

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

@kdy618 we should probably run prettier to fix the linting failure but this looks fine.

@kdy618
Copy link
Contributor Author

kdy618 commented Jan 19, 2021

@jofsky @MatthewBlanski , fixed the linter :D

Copy link
Contributor

@jofsky jofsky left a comment

Choose a reason for hiding this comment

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

Great work!

Copy link
Contributor

@MatthewBlanski MatthewBlanski left a comment

Choose a reason for hiding this comment

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

Happy path testing resolved successfully. The code and references look to be in order.

@spinkelman
Copy link
Contributor

QA Passes - Ran trial > trial_org - Verified picklist values appear in org and no error on task run in cci.
Waiting to merge until after release merge per Matt's comment above.

@jofsky jofsky merged commit 10ac5ef into main Jan 29, 2021
@jofsky jofsky deleted the feature/kdy/taskToAddCitizenshipStatusValue branch January 29, 2021 18:58
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.

[K12] Add task to K12 yml to support New Picklists Value for Citizenship Status on Contact
4 participants