Skip to content

Actualize snippet example for Next() Method with rest of languages #9759

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

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

chuve
Copy link
Contributor

@chuve chuve commented Mar 22, 2024

The example doesn't represent a description where it's embedded. The description states, generate a specific number of random numbers requested by the user.. However, the random numbers are hardcoded to 10 and aren't requested from user input.

Summary

The page where the example is embedded: https://learn.microsoft.com/en-us/dotnet/api/system.random.next?view=net-8.0

…sions.

The example doesn't represent a description of it, which has the statement, `generate a specific number of random numbers requested by the user.`. In the current example, the number of random numbers is hard coded to 10 and doesn't read from user input.
@chuve chuve requested a review from a team as a code owner March 22, 2024 16:17
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 22, 2024
@ghost ghost added the area-System.Runtime label Mar 22, 2024
@chuve
Copy link
Contributor Author

chuve commented Mar 22, 2024

@dotnet-policy-service agree

This comment was marked as outdated.

@gewarren
Copy link
Contributor

Thanks for noticing and fixing the discrepancy @chuve. It might be better to keep the original code (since it's simpler) and just change the intro text. What do you think? Since the example is more about Random that about reading user input.

@chuve
Copy link
Contributor Author

chuve commented Mar 25, 2024

Thanks for noticing and fixing the discrepancy @chuve. It might be better to keep the original code (since it's simpler) and just change the intro text. What do you think? Since the example is more about Random that about reading user input.

That was my first thought—update the intro text. However, after I checked code snippets in other languages (C++ and VB), I concluded that it would probably be better to update the code snippet instead. I agree with you that the original code is simpler and intended to show the use case of the Random.Next method.

@gewarren gewarren enabled auto-merge (squash) March 25, 2024 18:50
@chuve
Copy link
Contributor Author

chuve commented Mar 27, 2024

@gewarren, it seems like one of the gh actions has failed - https://github.com/dotnet/dotnet-api-docs/actions/runs/8393286204/job/23071411802?pr=9759
Do you know if I need to fix it? If so, how can I do it?
Thanks!

@gewarren
Copy link
Contributor

gewarren commented Mar 27, 2024

@gewarren, it seems like one of the gh actions has failed - https://github.com/dotnet/dotnet-api-docs/actions/runs/8393286204/job/23071411802?pr=9759 Do you know if I need to fix it? If so, how can I do it? Thanks!

@chuve I will fix this error in #9770. It involves adding a project file and then fixing all the snippets so they compile as part of one project. Once that PR is merged, we can close/reopen this one to rerun snippets 5000.

@gewarren gewarren closed this Mar 28, 2024
auto-merge was automatically disabled March 28, 2024 19:51

Pull request was closed

@gewarren gewarren reopened this Mar 28, 2024
Copy link

Learn Build status updates of commit f7a53dc:

✅ Validation status: passed

File Status Preview URL Details
snippets/csharp/System/Random/Overview/next3.cs ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@gewarren gewarren merged commit 394571c into dotnet:main Mar 28, 2024
@chuve chuve deleted the patch-1 branch April 2, 2024 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Runtime community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants