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

Multiple improvements to image service code and tests #1008

Merged
merged 4 commits into from
May 17, 2021

Conversation

paskal
Copy link
Collaborator

@paskal paskal commented May 17, 2021

These are changes from #992 which are not relevant to the purpose of that PR, cut into separate PR to simplify 992 code review.

  • rename variables according to golangci-lint recommendation
  • make consistent returns in bolt_store
  • less magic consonants in tests
  • make commitTTL equal to EditDuration (opposed to 1.5 previously), so that image is committed to permanent storage after the comment can no longer be edited.
  • move cleanupTTL to the Cleanup function, as it's not used elsewhere in the code.

I advise merging this with rebase.

@paskal paskal requested a review from umputun May 17, 2021 00:06
Copy link
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

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

looks ok, except the cleanupTTL confusion

backend/app/store/image/image.go Outdated Show resolved Hide resolved
So that image is committed to permanent
storage after comment can no longer be edited.

Also, move cleanupTTL to Cleanup function,
as it's not used elsewhere in the code.
@paskal paskal force-pushed the paskal/small_improvements branch from 17150fa to 15d93bc Compare May 17, 2021 06:51
Copy link
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

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

LGTM

@umputun umputun merged commit f9eb39d into master May 17, 2021
@umputun umputun deleted the paskal/small_improvements branch May 17, 2021 06:55
@paskal paskal added this to the v1.9 milestone Jan 15, 2022
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