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

Clarify meaning of “replacement indicator” in warning #3425

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

mart-e
Copy link
Contributor

@mart-e mart-e commented Nov 11, 2024

Not always clear what was the issue with only the keyword.

  • Ensured tests pass and (if applicable) updated functional test output
  • Conformed to code style guidelines by running appropriate linting tools
  • Added tests for changed code
  • Updated documentation for changed code

After writing a long article, I got this warning message but I had to look in the code to understand what was wrong (used {content} while I wanted {filename}). Not really clear what we meant by "Replacement Indicator".

Before this commit:

$ make devserver
…
Serving site at: http://127.0.0.1:8000 - Tap CTRL-C to stop
[14:46:30] WARNING  Replacement Indicator 'content' not recognized, skipping replacement                                                                                                           contents.py:360
Done: Processed 210 articles, 24 drafts, 0 hidden articles, 1 page, 1 hidden page and 0 draft pages in 3.53 seconds.

After this commit

$ make devserver
…
Serving site at: http://127.0.0.1:8000 - Tap CTRL-C to stop
[14:45:47] WARNING  Replacement Indicator 'content' not recognized in '{content}/2024/my-shinny-article.md', skipping replacement                                           contents.py:360
Done: Processed 210 articles, 24 drafts, 0 hidden articles, 1 page, 1 hidden page and 0 draft pages in 3.05 seconds.

An alternative patch could be simply:

- "Replacement Indicator '%s' not recognized, skipping replacement"
+ "Replacement Indicator '{%s}' not recognized, skipping replacement"

I guess I would have understood as well. Let me know what you prefer.

Not always clear what was the issue with only the keyword
@mart-e mart-e force-pushed the replacement-indicator-log-full branch from bcf84b4 to 31538d1 Compare November 11, 2024 13:54
@mart-e mart-e marked this pull request as ready for review November 11, 2024 13:56
@justinmayer justinmayer changed the title [IMP] add information in error message Clarify meaning of “replacement indicator” in warning Nov 27, 2024
Copy link
Member

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

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

Many thanks, Martin, for this clarification 👍

@justinmayer justinmayer merged commit ac28ddb into getpelican:main Nov 27, 2024
16 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.

2 participants