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

Document cyclical error in include in UG #2198

Merged
merged 10 commits into from
Mar 10, 2023

Conversation

yucheng11122017
Copy link
Contributor

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Documents #2177

Overview of changes:
Added a warning about the cyclical error in includes when variables have the same name. Included suggested solution for this bug.

Anything you'd like to highlight/discuss:

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)
Document cyclical error in include in UG


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

@damithc
Copy link
Contributor

damithc commented Mar 7, 2023

As this is a rare case (i.e., most users will not need to know about it), we should present it in a way most readers are not forced to read it but they can locate this information when they need to. Any ideas?
Perhaps a collapsed panel with a heading like A caveat, if you are recursive includes ... ?

@tlylt
Copy link
Contributor

tlylt commented Mar 7, 2023

Thank you @yucheng11122017 for taking this up. Could we:

  • add the explanation mentioned in Using include inside include results in cyclic references #2177 (comment) before the new panel to improve the current description, such that devs like yourself understand the rationale for such design
  • construct an "MVP" example such that we show the cause, the error (which you have already included), and the resolution (which is missing)

Copy link
Contributor

@tlylt tlylt left a comment

Choose a reason for hiding this comment

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

LGTM, fixed a typo and added line numbers for consistency within the panel.

@tlylt tlylt added this to the v4.1.1 milestone Mar 8, 2023
@tlylt tlylt changed the title Add documentation about cyclical error in includes in UG Document cyclical error in include in UG Mar 10, 2023
@tlylt tlylt merged commit 10b80ee into MarkBind:master Mar 10, 2023
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.

3 participants