Skip to content

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:

Removes includes of variables in UG due to #2211
Checked through the rest of UG and DG to see if there was any other instance of this use case.

Overview of changes:
Removes includes with variables and replaces it with the actual code.

Anything you'd like to highlight/discuss:

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)
Remove include of variables 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

Copy link
Contributor

@EltonGohJH EltonGohJH left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@lhw-1 lhw-1 left a comment

Choose a reason for hiding this comment

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

LGTM!

Should we update the UG (Includes section) to document this?

@yucheng11122017
Copy link
Contributor Author

LGTM!

Should we update the UG (Includes section) to document this?

Mmmm may be better to not reveal our weaknesses? :)

@damithc
Copy link
Contributor

damithc commented Mar 31, 2023

Mmmm may be better to not reveal our weaknesses? :)

It's OK to document limitations but should be done in a way that most readers don't need to read about it. Only those who are encountering this kind of use case should need to read about it.

@yucheng11122017
Copy link
Contributor Author

Mmmm may be better to not reveal our weaknesses? :)

It's OK to document limitations but should be done in a way that most readers don't need to read about it. Only those who are encountering this kind of use case should need to read about it.

Alright I will open a separate PR for this :)

@yucheng11122017 yucheng11122017 merged commit 463b7a0 into MarkBind:master Apr 1, 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.

4 participants