Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Collect Named CSS Region Flows#4890

Closed
JeffryBooher wants to merge 9 commits into
masterfrom
jeff/collect-region-flow-names
Closed

Collect Named CSS Region Flows#4890
JeffryBooher wants to merge 9 commits into
masterfrom
jeff/collect-region-flow-names

Conversation

@JeffryBooher

Copy link
Copy Markdown
Contributor
  • Adds a function to extract all named css region flow names.

Comment thread src/language/CSSUtils.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This function is actually always returning an array, even thought it can be empty. The ? doesn't seem to be required here.

@JeffryBooher

Copy link
Copy Markdown
Contributor Author

@TomMalbran we only have to do @Private when an object is exported with private methods. In this case we aren't exporting the functions so @Private isn't required. The other "private" functions in this file also have JSDoc that doesn't start with @Private.

@TomMalbran

Copy link
Copy Markdown
Contributor

Ok. So I am not sure if we are suing @Private always right then, the docs don't make it that clear either.

@ghost ghost assigned RaymondLim Aug 23, 2013
Comment thread src/language/CSSUtils.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: why one character indent for this line? Also extra space between "the" and "content."

@RaymondLim

Copy link
Copy Markdown
Contributor

Done with review. Only some minor nits.

Comment thread src/language/CSSUtils.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great! I was about to tell you to exclude "none" and "inherit". I'm not sure about others and reserved flow names.

Again, please use double-quotes for these strings.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants