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

[MRG+1] Startproject templates override #1575

Merged
merged 1 commit into from Dec 4, 2015

Conversation

@palego
Copy link
Contributor

@palego palego commented Oct 31, 2015

Hello Scrapy-makers :-)

This would be my first, modest, contribution to Scrapy, that I have been pleased to use on and off for a couple of years now.

This is an attempt to address the missing override of TEMPLATES_DIR at project creation described in issue #671.

I saw PR #1035, but it seems to be on a dead end, and the reasons for that seem to be:

  • it implemented multiple override levels in a way that is not compatible with the new override mechanism,
  • considering that there is no settings.py (let alone spiders) at startproject time, the only level of override is the command-line, so it was possibly a bit over-engineered.

With this in mind, this PR keeps to the simple route proposed in the issue and copies/pastes from genspider.py.

Looking forwards to your feedback.

@codecov-io
Copy link

@codecov-io codecov-io commented Oct 31, 2015

Current coverage is 82.89%

Merging #1575 into master will increase coverage by +0.01% as of 2dc97ae

Powered by Codecov. Updated on successful CI builds.

@kmike
Copy link
Member

@kmike kmike commented Nov 2, 2015

Looks good, +1 to merge. Could you please squash the commits?

@palego palego force-pushed the palego:startproject-templates-override branch from 4fbdea0 to fd2f3b8 Nov 2, 2015
@palego
Copy link
Contributor Author

@palego palego commented Nov 2, 2015

This way?

@kmike kmike changed the title Startproject templates override [MRG+1] Startproject templates override Nov 2, 2015
@kmike
Copy link
Member

@kmike kmike commented Nov 2, 2015

👍

@palego
Copy link
Contributor Author

@palego palego commented Nov 4, 2015

Finding now that the content in the first level of the templates_dir is ignored, except for scrapy.cfg.
I would find useful to add some reusable contents in there such as:

  • files that belong to the project root (i.e. .gitignore),
  • data directories that belong better out of the way of the code (like a csv feed output directory).

The change is pretty easily done by doing the copytree at the project root level, and then rename 'module' to project_name.

This would impact both the files that were touched by this PR (startproject.py & test_commands.py).

My question is about the workflow: should I update this PR?
Or should I make a new branch, based on this branch, as there would be a dependency on the test code?
Or is there a smarter way around this?

@palego
Copy link
Contributor Author

@palego palego commented Nov 12, 2015

Found https://github.com/codecov/support/wiki/Commit-Coverage-Status , but did not understand the slightest part of it.
Could someone give me a hint (be it explanation, action or documentation) about the codecov failure, I would be grateful.
EDIT: more specifically, the fact that docs are touched does not explain why over a dozen files, most untouched by this PR, are listed in the report.

@palego
Copy link
Contributor Author

@palego palego commented Nov 16, 2015

@kmike should I squash it all again? or is it too messed up at this point?

@kmike
Copy link
Member

@kmike kmike commented Nov 29, 2015

Hey @palego,

Sorry for the delay. Yeah, codecov thinks you've touched many files because scrapy master changed since the PR was opened. Rebasing on master should fix it. I think there is no need to squash, but it doesn't hurt either :)

The PR looks fine to me, +1 to merge it.

My main worry is that we probably shouldn't go too far improving scrapy template generating features; if it is hard to support IMHO it is better to switch to http://cookiecutter.readthedocs.org/en/latest/.

@palego
Copy link
Contributor Author

@palego palego commented Nov 30, 2015

Hi @kmike,

Nevermind the delay, I understand this is low priority.

Thanks for the explanation (sorry, I should have realized that by myself instead of wasting your time...). I will rebase/squash this evening.

Regarding template generation, I won't go any further. The purpose here is just to have the current mechanism working as documented, with no harm to a future cookiecutter implementation.

allow override of TEMPLATES_DIR for startproject
copy full TEMPLATES_DIR/project tree
doc update
@palego palego force-pushed the palego:startproject-templates-override branch from 340be36 to 2c25100 Dec 1, 2015
@palego
Copy link
Contributor Author

@palego palego commented Dec 1, 2015

Done.
And improving: now I pulled a strike ;-)
The test failures do not seem related to my changes though.

dangra added a commit that referenced this pull request Dec 4, 2015
[MRG+1] Startproject templates override
@dangra dangra merged commit 3881eaf into scrapy:master Dec 4, 2015
0 of 3 checks passed
0 of 3 checks passed
codecov/patch CI failed: coverage not measured fully.
Details
codecov/project CI failed: coverage not measured fully.
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@palego palego deleted the palego:startproject-templates-override branch Dec 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants