Skip to content

Add mergeCollection method to Onyx.update#155

Merged
danieldoglas merged 7 commits into
mainfrom
jules-addMergeCollectionToUpdate
Jul 4, 2022
Merged

Add mergeCollection method to Onyx.update#155
danieldoglas merged 7 commits into
mainfrom
jules-addMergeCollectionToUpdate

Conversation

@Julesssss

@Julesssss Julesssss commented Jul 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Enables the mergeCollection Onyx method. To be tested in the Web-E PR.

Details

Simply enables the mergeCollection Onyx method.

Related Issues

https://github.com/Expensify/Expensify/issues/214912

Automated Tests

Added a new test specifically to test a mergeCollection Onyx update

Linked PRs

@Julesssss Julesssss self-assigned this Jul 1, 2022
@github-actions

github-actions Bot commented Jul 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@Julesssss

Julesssss commented Jul 4, 2022

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@Julesssss Julesssss force-pushed the jules-addMergeCollectionToUpdate branch from 0c27b02 to 0bf39b3 Compare July 4, 2022 17:35
@Julesssss Julesssss requested a review from a team July 4, 2022 18:27
@Julesssss Julesssss marked this pull request as ready for review July 4, 2022 18:27
@melvin-bot melvin-bot Bot requested review from mountiny and removed request for a team July 4, 2022 18:27
mountiny
mountiny previously approved these changes Jul 4, 2022
View reviewed changes

@mountiny mountiny left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One very small nitty picky comment, but otherwise LGTM

Comment thread tests/unit/onyxTest.js Outdated
@Julesssss Julesssss requested a review from a team as a code owner July 4, 2022 20:25
@melvin-bot melvin-bot Bot requested review from danieldoglas and removed request for a team July 4, 2022 20:26
@mountiny mountiny self-requested a review July 4, 2022 20:39
mountiny
mountiny approved these changes Jul 4, 2022
View reviewed changes

@mountiny mountiny left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! Appreciate the change for better sleep 🙌

Did you assign other reviewer on purpose or was it gh blip? should we wait for Daniel with merging?

danieldoglas
danieldoglas approved these changes Jul 4, 2022
View reviewed changes

@danieldoglas danieldoglas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@danieldoglas danieldoglas merged commit 445769a into main Jul 4, 2022
@danieldoglas

danieldoglas commented Jul 4, 2022

Copy link
Copy Markdown
Contributor

This failed on publish @mountiny @Julesssss

@Julesssss

Julesssss commented Jul 7, 2022

Copy link
Copy Markdown
Contributor Author

This failed on publish @mountiny @Julesssss

Hey @danieldoglas. Sorry, I'm currently OOO due to Covid. What do you mean by failed on publish?

@mountiny

mountiny commented Jul 7, 2022

Copy link
Copy Markdown
Contributor

@Julesssss Hope you having mild symptoms!

I think he means this workflow failed https://github.com/Expensify/react-native-onyx/actions/runs/2612339269

I am not sure if it is fine to just retry, it might be realted to some of the env changes made recently

@mountiny

mountiny commented Jul 7, 2022

Copy link
Copy Markdown
Contributor

I will try and rerun it, what could go wrong

@Julesssss

Julesssss commented Jul 7, 2022

Copy link
Copy Markdown
Contributor Author

Thanks. That makes sense.

I retried, but the action failed because the tag already existed. Now attempting to delete the tag and re-run the action.

@Julesssss

Julesssss commented Jul 7, 2022

Copy link
Copy Markdown
Contributor Author

Seeing a Github authentication issue. Taking this to #developer

Screenshot 2022-07-07 at 11 32 59

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

Reviewers

@danieldoglas danieldoglas danieldoglas approved these changes

@mountiny mountiny mountiny approved these changes

Assignees

@Julesssss Julesssss

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Julesssss @danieldoglas @mountiny