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

Setttings: strip whitespace between entries #3161

Merged
merged 3 commits into from
Jun 11, 2024

Conversation

fliiiix
Copy link
Contributor

@fliiiix fliiiix commented May 21, 2024

It took me a lot of time to figure out why this config does not do what I would expect:

CSRF_TRUSTED_ORIGINS = "https://foo.bar.example/, http://foo.example/"

The first commit 6400003 addresses this and strips white space.
I feel like refactoring that out into helper functions a4e34ac makes it more readable, but i can also remove that commit if thats not in line with the project.
And i used black for formatting 03c10bc since i wasn't sure what the policy is here, but also in a extra commit which can be removed easily again.

I feel all these changes are useful but let me know :D

@CLAassistant
Copy link

CLAassistant commented May 21, 2024

CLA assistant check
All committers have signed the CLA.

@vabene1111
Copy link
Collaborator

Hi, thank you for the PR, I am very busy atm but will try to look into this as soon as possible.

@vabene1111
Copy link
Collaborator

ok so sorry for the long delay. The logic change looks good. May I ask you to do two things:

  1. revert the formatting, It was well intended but formatting is a very difficult topic that has not yet been 100% decided
  2. can you change the branch to merge into feature/vue3, because I don't want to make any setting changes on develop any more

thank you

@fliiiix fliiiix changed the base branch from develop to feature/vue3 June 11, 2024 10:25
This makes configurations valid which contain one or more
extra spaces.

Example:
CSRF_TRUSTED_ORIGINS = "https://foo.bar.example, http://foo.example"

Would be invalid.
@fliiiix
Copy link
Contributor Author

fliiiix commented Jun 11, 2024

revert the formatting, It was well intended but formatting is a very difficult topic that has not yet been 100% decided

I can highly recommend using black and stop discussing about it, worked well for me at work and in my open source projects,
but i removed it from this MR

can you change the branch to merge into feature/vue3
rebased on new branch

let me know if there is anything else required to get it merged :)

@vabene1111
Copy link
Collaborator

thank you. I know black works very well but my personal preference is in the way, I just simply dont like how it looks.

@vabene1111 vabene1111 merged commit 08309a6 into TandoorRecipes:feature/vue3 Jun 11, 2024
4 checks passed
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.

None yet

3 participants