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

Add documentation for entity declaration design work #4230

Merged
merged 6 commits into from
Aug 21, 2024

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Aug 19, 2024

Trying to pull in key elements of #3762, #3763, and #3980 (decl matching and extern, essentially). These aren't specific to any particular declaration type, but are common to entities, so suggesting a new doc oriented on that.

There's probably more that could be said here, I'm just focused on getting the recent formal discussion mirrored into the design.

@github-actions github-actions bot requested a review from josh11b August 19, 2024 23:42
@github-actions github-actions bot added the documentation An issue or proposed change to our documentation label Aug 19, 2024
docs/design/declaring_entities.md Outdated Show resolved Hide resolved
docs/design/declaring_entities.md Outdated Show resolved Hide resolved
docs/design/declaring_entities.md Outdated Show resolved Hide resolved
docs/design/declaring_entities.md Outdated Show resolved Hide resolved
docs/design/declaring_entities.md Outdated Show resolved Hide resolved
docs/design/declaring_entities.md Outdated Show resolved Hide resolved
docs/design/declaring_entities.md Outdated Show resolved Hide resolved
docs/design/declaring_entities.md Outdated Show resolved Hide resolved
Comment on lines 111 to 112
- This may be used to break dependency cycles between libraries, which
could be used to improve build parallelization.
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this might be alternatively be used to improve build scalability by reducing dependencies. This is independent of breaking dependency cycles, and isn't specifically about build parallelization.

Suggested change
- This may be used to break dependency cycles between libraries, which
could be used to improve build parallelization.
- This may be used to break dependency cycles between libraries, which
could be used to improve build parallelization.
- This may be used to reduce transitive dependencies, improving build
scalability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about this rephrasing... (to copy here)

    -   This can be used to improve build performance, such as by splitting out
        a declaration in order to reduce a library's dependencies.

Note, trying to step aside from specific ways it might improve build performance because I want these bullets to focus more on what the feature does, versus how it might be used.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with the new text, but don't you want something about that it can allow you to break cycles too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To me, that sort of feels like details where if we really want to explain how to use the feature this way, we should have more examples of refactorings to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i.e., I'm trying to figure out the right line to draw between proposal advocating for adoption of the feature, design trying to explain what it is, and user guide explaining how to apply it in anger.

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like the design shouldn't just say what it is, but also the why, and the purpose of the feature is definitely in scope.

@jonmeow jonmeow added this pull request to the merge queue Aug 21, 2024
Merged via the queue into carbon-language:trunk with commit 89be57f Aug 21, 2024
8 checks passed
@jonmeow jonmeow deleted the decl-doc branch August 21, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation An issue or proposed change to our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants