Skip to content

Split repository publication settings into two types#6776

Merged
shiftkey merged 25 commits intodevelopmentfrom
more-tabs-more-state
Feb 22, 2019
Merged

Split repository publication settings into two types#6776
shiftkey merged 25 commits intodevelopmentfrom
more-tabs-more-state

Conversation

@iAmWillShepherd
Copy link
Contributor

@iAmWillShepherd iAmWillShepherd commented Feb 5, 2019

Fixes #6546
Related to #6636.

This PR includes a refactor to better support #6636 with types. I've split the publication settings into two distinct types, DotcomPublicationSettings and GhePublicationSettings. This was done because we were previously sharing state between the two tabs which meant changes made in one tab would not persist once another tab was selected.


From original PR

cc @brendonbarreto

Release notes

Notes: [Fixed] Publish settings remembered when switching between publish targets

@iAmWillShepherd iAmWillShepherd added the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Feb 5, 2019
Copy link
Member

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

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

Haven't had a chance to test this out, but I wanted to call out some suggestions to be consistent with other areas.

@shiftkey
Copy link
Member

shiftkey commented Feb 7, 2019

I've opened #6789 targeting this branch to uncover why Travis is failing, and instead I see the Pipelines and CircleCI builds (which seem to run for every branch) are also failing in the same way. I've opened #6790 so I can dig into this further.

@shiftkey shiftkey force-pushed the more-tabs-more-state branch from 445d746 to a75a9aa Compare February 7, 2019 20:52
@shiftkey shiftkey force-pushed the more-tabs-more-state branch from a75a9aa to 14a82ef Compare February 7, 2019 21:23
@shiftkey shiftkey removed the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Feb 8, 2019
@shiftkey shiftkey self-assigned this Feb 8, 2019
@shiftkey
Copy link
Member

shiftkey commented Feb 8, 2019

Please hold off on merging this PR - I want to use this branch to see if I can uncover why our integration tests are now failing.

@shiftkey shiftkey force-pushed the more-tabs-more-state branch 15 times, most recently from 51de2d0 to b4d5fed Compare February 8, 2019 19:24
@shiftkey shiftkey force-pushed the more-tabs-more-state branch 2 times, most recently from 8c44947 to a5bdea9 Compare February 21, 2019 18:03
@shiftkey shiftkey force-pushed the more-tabs-more-state branch from a5bdea9 to 33e51b1 Compare February 21, 2019 18:04
Copy link
Member

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

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

A bit of cleanup to make this new code easier to read, otherwise LGTM.

@shiftkey shiftkey self-assigned this Feb 21, 2019
shiftkey and others added 3 commits February 21, 2019 13:26
Co-Authored-By: iAmWillShepherd <iAmWillShepherd@users.noreply.github.com>
Co-Authored-By: iAmWillShepherd <iAmWillShepherd@users.noreply.github.com>
Co-Authored-By: iAmWillShepherd <iAmWillShepherd@users.noreply.github.com>
@shiftkey shiftkey merged commit 1cf334b into development Feb 22, 2019
@shiftkey shiftkey deleted the more-tabs-more-state branch February 22, 2019 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants