Skip to content

Conversation

@herra
Copy link

@herra herra commented May 28, 2020

Change made because of django csrf validation

@herra herra requested review from groupsky and i-trofimtschuk May 28, 2020 11:13
@herra herra changed the base branch from master to feature/fe2 May 28, 2020 11:13
@lgtm-com
Copy link

lgtm-com bot commented May 28, 2020

This pull request fixes 2 alerts when merging 5788853 into bbe3317 - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable
  • 1 for Unused variable, import, function or class

Copy link

@groupsky groupsky left a comment

Choose a reason for hiding this comment

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

It is a bad idea to use GET for operations that are modifying - caching can prevent the request to reach server and goes against the implicit meaning of http verbs

@dkusnik
Copy link
Contributor

dkusnik commented May 29, 2020

I would also prefer GET instead of POST. here is no body content, so whats the purpose of POST? lternatively DELETE would fit, all those requests remove some resources.

@groupsky groupsky marked this pull request as draft June 29, 2020 10:09
@i-trofimtschuk i-trofimtschuk removed their request for review October 19, 2020 06:56
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.

3 participants