@diosmosis opened this Pull Request on February 12th 2021 Member

Description:

Noticed calling getDoneStringFlagFor w/ $plugin = false when pluginOnly=1 can result in done flags like done..

Review

  • [ ] Functional review done
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@tsteur commented on February 14th 2021 Member

@diosmosis would this be causing reading archives that were created with this flag? or would that basically not have worked already anyway?

@diosmosis commented on February 14th 2021 Member

@tsteur if there were already archives made w/ done., it might be an issue... we could have an update to change those I guess (an update where name = 'done.'?

@tsteur commented on February 14th 2021 Member

@diosmosis could be done. Just trying to understand: Would it be otherwise an issue to having flags with done.? But all good to add such an update script and fixing it

@diosmosis commented on February 14th 2021 Member

Would it be otherwise an issue to having flags with done.?

For leaving them, I think the only issue is if they're no longer queried and done archives aren't present. Now it might be an issue in that we might not be querying them properly? It's hard to tell w/ this issue exactly where it causes problems.

@diosmosis commented on February 14th 2021 Member

Note: I did see them created in prod when making plugin specific archives, maybe nowhere else.

@tsteur commented on February 14th 2021 Member

Also quickly another question: Would it cause issues if there are afterwards maybe two archives with a done flag? And they might contain different reports? I know it's a bit hard to say what would happen. I could imagine that some archive may be deleted or reports would be only read from one archive?

@diosmosis commented on February 14th 2021 Member

Would it cause issues if there are afterwards maybe two archives with a done flag?

I'm not sure what you mean? After the update, having two archives w/ done should be fine since that's what we want (all archives would have the done flag or done w/ plugin/segment.

@tsteur commented on February 14th 2021 Member

All good then 👍

@diosmosis commented on February 15th 2021 Member

merging this, fyi @tsteur

This Pull Request was closed on February 15th 2021
Powered by GitHub Issue Mirror