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.

Sign up for GitHub

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

Jump to bottom

simpler archive/unarchive errors #2191

Merged
merged 8 commits into from May 20, 2020
Merged

simpler archive/unarchive errors #2191

merged 8 commits into from May 20, 2020

Conversation

opqdonut
Copy link
Contributor

follow-up to #2178

kinda part of #2052

Definition of Done / Review checklist

Reviewability

  • link to issue

API

  • API is backwards compatible or completely new

Documentation

  • update changelog if necessary

Testing

  • complex logic is unit tested
  • valuable features are integration / browser / acceptance tested automatically

Follow-up

  • no critical TODOs left to implement

Macroz
Macroz approved these changes May 20, 2020
View reviewed changes
Copy link
Collaborator

@Macroz Macroz left a comment

Choose a reason for hiding this comment

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

I'm not sure the error messages are very clear to the user but the code is good.

@opqdonut
Copy link
Contributor Author

Yeah I think the error texts are a bit worse now. However this is an admin feature so it doesn't need to be quite as polished. Ideas?

@opqdonut
Copy link
Contributor Author

archive-failed
unarchive-failed

@opqdonut
Copy link
Contributor Author

Now it looks like this:
archive-failed-2
unarchive-failed-2

@opqdonut opqdonut merged commit c8a647a into master May 20, 2020
@opqdonut opqdonut deleted the error-refactor branch May 20, 2020 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Macroz Macroz Macroz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@opqdonut @Macroz