Skip to content

Conversation

@VeldaKiara
Copy link
Contributor

The change is an add-on in case the default value of this line default(bag(bag_name, alt_default, key) fails which is what was causing the None entry on docs.

It eliminates having None on this issue and on other future news entries.

Fixes issue 622

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

Copy link
Contributor

@johnzhou721 johnzhou721 left a comment

Choose a reason for hiding this comment

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

Hint: If I'm not mistaken, the strings for such things is at databags folder. Can you check things at places with things that's similar to the other fields, since there probably should be a word at the position of None?

@johnzhou721
Copy link
Contributor

johnzhou721 commented May 19, 2025

@freakboy3742 Hope that this hint doesn't give away too much stuff.

Clarification (might contain spoilers, but I tried not to) I didn't look at those files (line-by-line / search) yet (but I do know that it stores the strings from when I first looked at and scrolled through the source of the website). My wording in the review was sort of vague, but I don't know specifically and don't want to know specifically what's going on (so other contribers can do more digging), but there probably should be a word at the place of None (and I suspected that there might be a field holding it in the databags) so that's why I left that comment.

I just reworded my comment to be more clear.

@freakboy3742 freakboy3742 added the preview Approved for an automated preview label May 20, 2025
@github-actions
Copy link

Visit the preview URL for this PR (updated for commit 8b55c53):

https://beeware-org--pr625-lektor-j3fkbigc.web.app

(expires Tue, 27 May 2025 02:21:44 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: b0da44bc067e7d9a4255c77cb2c5fce572218cec

@johnzhou721
Copy link
Contributor

@freakboy3742 This is a little bit not right... I broadly know where the issue is but as I mentioned in the clarification I'm refusing to dig deeper (so I can leave this issue to the sprinters to figure out).

@VeldaKiara
Copy link
Contributor Author

Checking on the above suggestions made

@VeldaKiara
Copy link
Contributor Author

@johnzhou721 I just saw why the statement was resulting in none

However, I would still suggest having a second fallback. I am pushing the change in a bit based on the above suggestions.

@VeldaKiara VeldaKiara requested a review from johnzhou721 May 20, 2025 03:39
@johnzhou721
Copy link
Contributor

johnzhou721 commented May 20, 2025 via email

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the fix!

@freakboy3742 freakboy3742 merged commit 0220f7e into beeware:lektor May 20, 2025
@johnzhou721
Copy link
Contributor

johnzhou721 commented May 20, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview Approved for an automated preview

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants