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

Fixed a bug in old assets deletion. #6

Closed
wants to merge 1 commit into from

Conversation

Maxzor
Copy link

@Maxzor Maxzor commented Aug 24, 2020

yes
well actually you might want to pull from simontom

@Maxzor Maxzor closed this Aug 24, 2020
@Maxzor Maxzor reopened this Aug 24, 2020
@WebFreak001
Copy link
Owner

with what data did the bug happen? There is no advantage in comparing string literals, it only adds bugs in case of stuff like DST changes or different timezones. If it was some undefined/null issue, that should be handled instead of being obfuscated by string comparision (where it would currently also crash in case of a null a.created_at)

@Maxzor
Copy link
Author

Maxzor commented Aug 24, 2020

You compare how you want, but currently the newest versions are deleted, not the oldest

@WebFreak001
Copy link
Owner

you are right, thanks for noticing that

@Maxzor
Copy link
Author

Maxzor commented Aug 24, 2020

np!
simontom is a bad guy he diverged, fixed it your way(if Date() does the job actually), and did not upstream
https://github.com/simontom/deploy-nightly

@WebFreak001
Copy link
Owner

for discussion about these changes see #4, I decided not to take these changes as-is because I did not agree with the API design, but I haven't gotten around to implement it myself yet.

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