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

Remove references to the flag-for-reply workflow. #225

Merged
merged 2 commits into from Jun 17, 2021

Conversation

rmol
Copy link
Contributor

@rmol rmol commented May 19, 2021

Status

Ready for review

Description of Changes

Updates the docs after removal of the flag-for-reply workflow in freedomofpress/securedrop#5954.

Testing

Not much to check with deletions, but I am interested in whether others think we should retain the API endpoint documentation. I decided it was just confusing, since the endpoint now does nothing.

Release

This should be merged after freedomofpress/securedrop#5954.

Checklist (Optional)

  • Doc linting (make docs-lint) passed locally
  • Doc link linting (make docs-linkcheck) passed
  • You have previewed (make docs) docs at http://localhost:8000

@eloquence
Copy link
Member

eloquence commented May 28, 2021

LGTM in principle once the main PR lands.

I am interested in whether others think we should retain the API endpoint documentation. I decided it was just confusing, since the endpoint now does nothing.

As a matter of good practice, it may be worth noting in its own section, such as "Deprecated functionality", explaining that the no-op endpoint and the is_flagged property will be removed in a future release. Right now, the presence of the is_flagged property in the documentation is a bit confusing, given that it is always false after the main PR lands.

@eloquence eloquence added this to Ready for Review in SecureDrop Team Board Jun 7, 2021
@rocodes rocodes self-requested a review June 17, 2021 16:47
Copy link
Contributor

@rocodes rocodes left a comment

Choose a reason for hiding this comment

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

LGTM. The only thing I would maybe suggest is that there are a couple sample JSON responses that include is_flagged: false and it could be good to add a comment (even inline in the code block) explaining that this is a legacy field that can be ignored.

e.g. https://github.com/freedomofpress/securedrop-docs/pull/225/files#diff-466879439de016d374b3dc3cc22f5e587900a70cfdd5c80a1e51e35227510419L137-L141

@eloquence
Copy link
Member

I've added a section to the end for now - @rocodes ready for another look from you

Copy link
Contributor

@rocodes rocodes left a comment

Choose a reason for hiding this comment

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

WFM. Thank you @eloquence

@rocodes rocodes merged commit 3b942f3 into main Jun 17, 2021
SecureDrop Team Board automation moved this from Ready for Review to Done Jun 17, 2021
@rocodes rocodes deleted the remove-flag-for-reply branch June 17, 2021 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants