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

Channels.archive #12

Open
wants to merge 54 commits into
base: master
Choose a base branch
from
Open

Channels.archive #12

wants to merge 54 commits into from

Conversation

chrisjoyce911
Copy link

Archiving and UnArchiving

Chris Joyce and others added 30 commits August 25, 2018 20:41
Was
 POST https://rocketchat.server/api/v1/login HTTP/1.1 1 1 map[Content-Type:[application/json]] {password=PASSWORD&user=USER}

Now
 POST https://rocketchat.server/api/v1/login HTTP/1.1 1 1 map[Content-Type:[application/json]] {{"username":"USER","password":"PASSWORD"}}
Using multi pagkages makes forked development very hard,
moved things round to have all in single pagkage

* Added Doer - can pass this in
* REST channels has good coverage
* REST information has good coverage
Added missing tests for REST
Included error tests
Added missing tests for REST
Included error tests
Renamed channels tests
Extended the example to include a PostMessage
Most client methods are used by other func during tests
Very simple tests only
Login and Logout
I have left some dead/unsed code that needs cleanup
All tests are passing :-)
Included testing
NOt fully happy with it but its a start
Missed this :-)
Creates a new public channel
Including simple test
Removes the channel from the user’s list of channels.
Included simple test
Archives a channel.
Unarchives a channel.
@geekgonecrazy
Copy link
Collaborator

All these PR's are based on your previous PR's. Makes this extremely hard to review.

Ideally when doing this sort of thing you keep your master branch completely the same as our master branch. Then you checkout from master and do a feature and open a PR. Then if doing another feature you switch to master, and checkout again from master.

This way when you open the PR your PR contains isolated changes.

This makes it much easier to review. We could also merge the easy ones like this one and your other two channel based ones quickly instead of having to try to figure out which one to review first. :)

@graywolf336
Copy link
Contributor

Agreed with @geekgonecrazy, I honestly am having trouble understanding everything that has happened

@chrisjoyce911
Copy link
Author

I have created another pull request that contains a change. (#13)

I don't have time at the moment to roll my master branch back but will to make it easer for reviews.
I will close other pull's at the moment

Included testing
NOt fully happy with it but its a start
Creates a new public channel
Including simple test
Removes the channel from the user’s list of channels.
Included simple test
Archives a channel.
Unarchives a channel.
Archives a channel.
Unarchives a channel.
Archives a channel.
@geekgonecrazy
Copy link
Collaborator

Are you able to resolve the conflicts here? Would be good to review and get this added. Others might find this useful

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ chrisjoyce911
❌ Chris Joyce


Chris Joyce seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

4 participants