Skip to content

[WIP] Bugfix/list revisions activation suffix #42

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

Conversation

Bockit
Copy link
Contributor

@Bockit Bockit commented Nov 11, 2015

My previous pull request added support for the idea of an activationSuffix config option, so it didn't always have to be current. Allowing you to theoretically have many activations in one db.

What I didn't take into account was fetchRevisions. This PR adds support for activationSuffix to fetchRevisions so the current revision is derived from activationSuffix.

That has been dealt with in this PR, the WIP part is: I still haven't taken into account revision trimming with multiple activated revisions. At the moment it will trim based on your activationSuffix, so if you had activated with activationSuffix currentSpecial in the past, you could lose it in the future if you are uploading with trimming and a different activationSuffix.

Any suggestions here? You could theoretically have a key listing activation keys/suffixes but maybe there's a better way.

@Bockit
Copy link
Contributor Author

Bockit commented Nov 11, 2015

Also whoever wrote the tests I've modified, it would be good if someone could make sure I haven't inadvertently broken something.

@ghedamat
Copy link
Contributor

ghedamat commented Dec 9, 2015

@Bockit sorry for the delay,

I think this is still interesting and worth finishing because if I get it correctly the current behaviour on master is partially broken (correct me if I'm wrong)

mind rebasing and then we can have another look?

@ghedamat
Copy link
Contributor

@Bockit ping

@Bockit
Copy link
Contributor Author

Bockit commented Jan 19, 2016

Yep, you're correct. I'll rebase tonight. Sorry, this got lost in all the noise.

@ghedamat
Copy link
Contributor

ghedamat commented Mar 1, 2016

@Bockit sorry for nudging you again.

If that's easier we could try to take this over and bring it to completion.

thanks!

@Bockit
Copy link
Contributor Author

Bockit commented Mar 7, 2016

@ghedamat Being real about this, I doubt I'm going to get much time to work on it in the short to medium term. We don't use this at work anymore and in my own time I'm doing other things. If others want to try and take over I welcome it.

@ghedamat ghedamat closed this Mar 21, 2016
@ghedamat
Copy link
Contributor

Will close for now

If the use case comes up again we can resume the work done here and take it to the finish line

thanks anyway @Bockit !

hope you this wasn't the reason for you stopping using ecd :D

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