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

Slightly rephrase and actualize -preview=in section #2937

Merged
merged 1 commit into from
Feb 22, 2021

Conversation

Geod24
Copy link
Member

@Geod24 Geod24 commented Feb 7, 2021

Saw #2916 and realize it needed some tiny adjustments w.r.t. -preview=in.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @Geod24!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@Geod24
Copy link
Member Author

Geod24 commented Feb 7, 2021

Screen Shot 2021-02-07 at 23 30 18

This could probably use a bit more "$(P)" to look better.

@Geod24
Copy link
Member Author

Geod24 commented Feb 7, 2021

Screen Shot 2021-02-08 at 0 57 45

spec/function.dd Outdated
Dynamic arrays$(COMMA) classes$(COMMA) associative arrays$(COMMA) function pointers$(COMMA) and delegates
will always be passed by value$(COMMA) to allow for covariance.
If the type of the parameter does not fall in one of those categories$(COMMA)
will always be passed by value$(COMMA).)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
will always be passed by value$(COMMA).)
will always be passed by value.)

Also remove other trailing commas, since each paragraph must end with full stop.

whether or not it is passed by reference is implementation defined$(COMMA) and the backend is free
to choose the method that will best fit the ABI of the platform.
$(B Note: This requires the $(D -preview=in) switch$(COMMA) available in v2.094.0 or higher.)
)
Copy link
Member

Choose a reason for hiding this comment

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

Wrap this paragraph in $(IMPLEMENTATION_DEFINED macro

spec/function.dd Outdated
@@ -1282,18 +1282,20 @@ $(H3 $(LNAME2 param-storage, Parameter Storage Classes))

$(H3 $(LNAME2 in-params, In Parameters))

$(B Note: The following requires the $(D -preview=in) switch$(COMMA) available in v2.094.0 or higher.
Copy link
Member

Choose a reason for hiding this comment

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

- Adding 'also' to connect it better with the previous sentence about 'const scope';
- Remove 'they' which was grammatically incorrect;
- Use parenthesis around 'e.g.', to be consistent with the two previous usages;
- Remove 'to allow for covariance, as 'in' are not covariant with const anymore;
- Added more `$(P)` for the paragraph too look less beefy;
- Make the note about the preview switch more prevalent not to confuse users;
- Document the behavior without the preview switch;
@Geod24
Copy link
Member Author

Geod24 commented Feb 22, 2021

[2021-02-22 07:02:46.140775] dautotest: Fetching from origin (refs/pull/2937/head -> refs/digger/pull/2937) ...
From git://github.com/dlang/dlang.org
 + d56766e1...02f2ad71 refs/pull/2937/head -> refs/digger/pull/2937  (forced update)
[2021-02-22 07:02:47.175555] dautotest: Getting URL https://api.github.com/repos/dlang/dlang.org/pulls/2937
[2021-02-22 07:02:47.657842] dautotest:  > Cache miss; ratelimit: 4491/5000
[2021-02-22 07:02:47.662864] dautotest: Git    commit SHA1: 02f2ad710e4c78bf176161fa8bddc1ad69acfdf8 (base: b7e5f5c21d8aa8ff042688cd7b828e3ca213e651)
[2021-02-22 07:02:47.662939] dautotest: GitHub commit SHA1: 02f2ad710e4c78bf176161fa8bddc1ad69acfdf8
[2021-02-22 07:02:47.662979] dautotest: Merging...
[2021-02-22 07:02:47.663026] dautotest: Merging dlang.org commits b7e5f5c21d8aa8ff042688cd7b828e3ca213e651..02f2ad710e4c78bf176161fa8bddc1ad69acfdf8
[2021-02-22 07:02:47.663086] dautotest: Cleaning repository dlang.org...
HEAD is now at b7e5f5c2 Merge pull request #2955 from MartinNowak/merge_stable
[2021-02-22 07:02:47.759826] dautotest: Error during repository cleanup.
[2021-02-22 07:02:47.759867] dautotest: Nuking /home/dtest/DAutoTest/work/repo/dlang.org...
[2021-02-22 07:02:47.767319] dautotest: Merge error: Failed to stat file `/home/dtest/DAutoTest/work/repo/dlang.org/.generated/druntime-2.095.1/test/coverage/src-merge.hst.exp'
[2021-02-22 07:02:47.767345] dautotest: Error: Merge failed

@CyberShadow : I think I'm not to blame for this one ?

@Geod24
Copy link
Member Author

Geod24 commented Feb 22, 2021

Fixed

@RazvanN7 RazvanN7 merged commit 388f49a into dlang:master Feb 22, 2021
@Geod24 Geod24 deleted the input-params branch February 22, 2021 11:41
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