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

Rename issue #3039 #3048

Merged
merged 7 commits into from Apr 18, 2018
Merged

Rename issue #3039 #3048

merged 7 commits into from Apr 18, 2018

Conversation

@Knapstad
Copy link
Contributor

@Knapstad Knapstad commented Apr 17, 2018

Pull Request Checklist

  • I’ve read the guidelines for contributing.
  • I updated AUTHORS.txt and CHANGES.txt (if the change is non-trivial) and documentation (if applicable).
  • I tested my changes.

Description

@Knapstad
Copy link
Contributor Author

@Knapstad Knapstad commented Apr 17, 2018

Rename DISABLE_INDEXES_PLUGIN_* settings #3039

Copy link
Contributor

@felixfontein felixfontein left a comment

DISABLE_INDEXES_PLUGIN_INDEX_AND_ATOM_FEED is still mentioned in docs/creating-a-site.rst and docs/sphinx/creating-a-site.rst

AUTHORS.txt Outdated
@@ -1,4 +1,4 @@
* `Agustin Henze <https://github.com/agustinhenze>`_
* `Agustin Henze <https://github.com/agustinhenze>`_

This comment has been minimized.

@felixfontein

felixfontein Apr 17, 2018
Contributor

You inserted a UTF-8 BOM here (a hexdump shows that the file now starts with ef bb bf).

This comment has been minimized.

@Kwpolska

Kwpolska Apr 17, 2018
Member

I fixed this for now. In the future, please make sure your editor does not insert BOMs (never use notepad.exe to edit stuff like that)

https://en.wikipedia.org/wiki/Byte_order_mark

Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Copy link
Member

@Kwpolska Kwpolska left a comment

Could you please add warnings for both options, like we do on nikola.py line 714 for UNSLUGIFY_TITLES?

self.config['DISABLE_INDEXES'] = self.config['DISABLE_INDEXES_PLUGIN_INDEX_AND_ATOM_FEED']

if 'DISABLE_INDEXES_PLUGIN_RSS_FEED' in self.config:
utils.LOGGER.warn('The DISABLE_INDEXES_PLUGIN_RSS_FEED setting was renamed DISABLE_MAIN_RSS_FEED')

This comment has been minimized.

@Kwpolska

Kwpolska Apr 17, 2018
Member

You forgot one line here, but I’m going to add it myself.

@Kwpolska Kwpolska requested a review from felixfontein Apr 17, 2018
Copy link
Contributor

@felixfontein felixfontein left a comment

LGTM!

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Apr 18, 2018

Thanks for contributing, @Knapstad!

@Kwpolska Kwpolska merged commit 8b15b50 into getnikola:master Apr 18, 2018
3 checks passed
3 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Knapstad
Copy link
Contributor Author

@Knapstad Knapstad commented Apr 18, 2018

@Kwpolska thank you for letting me :) and thanks for your patiens, its my first contribution out side my own projects

@Knapstad Knapstad deleted the Knapstad:Rename_Disable_Indexes branch Apr 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants