kpi-web-guild / django-girls-blog-slsh1o

django-girls-blog-slsh1o created by GitHub Classroom
MIT License
0 stars 0 forks source link

Create a skeleton of a Django project #21

Closed slsh1o closed 4 years ago

slsh1o commented 4 years ago

Init a first Django app and fix all raised by linters issue.

Need this commits on master branch to proceed with tutorial.

Ask for review also: @BogdanShevchenkoo @ZVitaliy @dark-lord671

slsh1o commented 4 years ago

@webknjaz ahh, yes. As you've said build failed. Need help with it. Received this error on travis-ci:

pylint...................................................................Failed
- hook id: pylint
- exit code: 1
Executable `pylint` not found
webknjaz commented 4 years ago

@slsh1o the current setup doesn't install pylint. local means that it hits the current env where pre-commit itself is installed so it should contain pylint too. I'd like to see that change in a separate PR.

webknjaz commented 4 years ago

Also some additional ignorecase and args was added to .gitignore and

This is not really related to adding a skeleton, is it? If you want it, put it in a separate PR. Although, I don't really like to customize gitignore.io template because if you want to update it from upstream you may lose extra changes. Are you sure you can't generate a template that contains it in that service?

.pre-commit-config.yaml to fix C0415, C0103 and E501.

So here's the thing: before this commit, those errors don't exist, so applying this commit does not fix them in master as you claim it does. Which makes it something that doesn't add any value but adds confusion because you've created a commit that's already clean and there's no way to actually see those errors that you claim to exist.

webknjaz commented 4 years ago

some additional ignorecase and args was added

Also, this is totally non-concrete: how am I supposed to guess what you changed by reading this?

slsh1o commented 4 years ago

Ok. Another try from scratch but on the same PR :)

slsh1o commented 4 years ago

Now should be ok but I still need #24 on master to pass Travis test here.

webknjaz commented 4 years ago

Make sure to close&re-open this PR after merging #24. This will re-trigger CI builds.

webknjaz commented 4 years ago

Plz don't make foxtrot merges