Skip to content

Comments

Revert global team conversations PRs #2862 and #2753#2877

Closed
smatting wants to merge 3 commits intodevelopfrom
revert-2862-elland/global-team-conv-refine
Closed

Revert global team conversations PRs #2862 and #2753#2877
smatting wants to merge 3 commits intodevelopfrom
revert-2862-elland/global-team-conv-refine

Conversation

@smatting
Copy link
Contributor

@smatting smatting commented Nov 28, 2022

Reverts #2862 and #2753

@smatting smatting temporarily deployed to cachix November 28, 2022 15:48 Inactive
@smatting smatting temporarily deployed to cachix November 28, 2022 15:48 Inactive
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Nov 28, 2022
@smatting smatting temporarily deployed to cachix November 28, 2022 15:54 Inactive
@smatting smatting temporarily deployed to cachix November 28, 2022 15:54 Inactive
@smatting smatting changed the title Revert "Improve global team conversation handling and self conversation creation error." Revert global team conversations PRs #2862 and #2753 Nov 28, 2022
repo = "mls-test-cli";
sha256 = "sha256-FjgAcYdUr/ZWdQxbck2UEG6NEEQLuz0S4a55hrAxUs4=";
rev = "82fc148964ef5baa92a90d086fdc61adaa2b5dbf";
sha256 = "sha256-/XQ/9oQTPkRqgMzDGRm+Oh9jgkdeDM1vRJ6/wEf2+bY=";
Copy link
Contributor

Choose a reason for hiding this comment

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

@smatting , it seems wrong to revert the version of mls-test-cli to use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess one of the PRs updated the mls-test-cli tool

zUser :: UserId -> Request -> Request
zUser = header "Z-User" . toByteString'

zClient :: ClientId -> Request -> Request
Copy link
Contributor

Choose a reason for hiding this comment

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

The absence of this is likely to fail integration test compilation.

Copy link
Contributor

@mdimjasevic mdimjasevic left a comment

Choose a reason for hiding this comment

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

Generally it looks good, but I think we can and should keep the latest mls-test-cli tool version used.

zBot :: UserId -> Request -> Request
zBot = header "Z-Bot" . toByteString'

zClient :: ClientId -> Request -> Request
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I probably confused that other instance of zClient with this one.


module Test.Galley.Intra.User where

-- import Debug.Trace (traceShow)
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to bring this back.

Copy link
Contributor Author

@smatting smatting Nov 28, 2022

Choose a reason for hiding this comment

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

I'm trying not to make any surgical choices in this PR, just a basic revert. When we re-apply the GTC PRs this line will likely get removed again

@smatting smatting temporarily deployed to cachix November 28, 2022 16:21 Inactive
@smatting smatting temporarily deployed to cachix November 28, 2022 16:21 Inactive
@elland
Copy link
Contributor

elland commented Nov 29, 2022

I'm wondering if it wouldn't be simpler/easier to just disable the endpoints for now? 🤔

@elland
Copy link
Contributor

elland commented Nov 29, 2022

We've decided to just comment out the endpoints for the release instead.

@elland elland closed this Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants