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

3408clean hook commit - WIP #3749

Closed
wants to merge 1 commit into from
Closed

3408clean hook commit - WIP #3749

wants to merge 1 commit into from

Conversation

JohnMcLear
Copy link
Member

@JohnMcLear JohnMcLear commented Mar 23, 2020

Clean PR for the hook from https://github.com/ether/etherpad-lite/pull/3408/files

Note that there are no docs included for this..

Seperate PRs for other features from that PR inbound.

@JohnMcLear JohnMcLear changed the title 3408clean commit 3408clean commit - Ready for Review / Merge @muxator Mar 23, 2020
@JohnMcLear JohnMcLear changed the title 3408clean commit - Ready for Review / Merge @muxator 3408clean hook commit - Ready for Review / Merge @muxator Mar 23, 2020
@muxator
Copy link
Contributor

muxator commented Mar 23, 2020

Thanks John, if possible do the development work in a private repo. I'll have to force push this.

@JohnMcLear
Copy link
Member Author

Yea, for now jus tmerge these ones in. I have to work on ether/ at the moment because I'm doing so much around saucelabs and those tests don't run from johnmclear -- perhaps I could set them up to run from johnmclear after this sprint is over

@JohnMcLear
Copy link
Member Author

@muxator assigned to me to recreate on my personal and create PR from there?

@muxator
Copy link
Contributor

muxator commented Mar 23, 2020

What's the rationale for this? Reading from #3408 it seems that:

Added this hook to run custom functions in timeline view

If it is so, this PR should be augmented with documentation (and tests maybe? I do not know if they exists).

The right time for doing it is now, before merging.

Edit: is this PR a complete replacement for #3408? If so can we safely close it? If there were more features, let's keep it open.

@muxator
Copy link
Contributor

muxator commented Mar 23, 2020

@muxator assigned to me to recreate on my personal and create PR from there?

Assigned just for taking note of who did what. It's not important.

The requests for clarifications and for documentation are real (also, if this PR completely replaces #3408 or is if just a part of it).

Thanks

@JohnMcLear
Copy link
Member Author

Yea whoops, need to remember how to do docs...

@JohnMcLear JohnMcLear changed the title 3408clean hook commit - Ready for Review / Merge @muxator 3408clean hook commit - WIP Mar 23, 2020
@JohnMcLear JohnMcLear closed this Mar 24, 2020
@muxator
Copy link
Contributor

muxator commented Mar 24, 2020

Replaced by #3757

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.

2 participants