Skip to content

Replaced >= with > in condition. #14825

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 16, 2019
Merged

Replaced >= with > in condition. #14825

merged 1 commit into from
Oct 16, 2019

Conversation

pavinan
Copy link
Contributor

@pavinan pavinan commented Oct 8, 2019

Summary of the changes

  • Replaced >= with > in condition.

@rynowak
Copy link
Member

rynowak commented Oct 8, 2019

@pavinan - is there a bug explaining what's wrong?

@pavinan
Copy link
Contributor Author

pavinan commented Oct 8, 2019

@rynowak No, It just caught my eye while I am looking around for something.

superdevguy
superdevguy approved these changes Oct 8, 2019
View reviewed changes
Copy link

@superdevguy superdevguy left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@Pilchie Pilchie added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Oct 8, 2019
javiercn
javiercn reviewed Oct 16, 2019
View reviewed changes
Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

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

Based on the comments the change seems harmless, it would be good if the PR includes a comment with a link to the appropriate documentation that states that flags.Length will always be greater than 0.

@pavinan
Copy link
Contributor Author

pavinan commented Oct 16, 2019

@javiercn Documentation states that if flags.Length is 0 then attribute itself is not emitted.
It is specified optimizations section.

https://github.com/dotnet/roslyn/blob/master/docs/features/nullable-metadata.md#optimizations

@javiercn
Copy link
Member

@pavinan Thanks, that's the piece of data I was missing.

@pranavkm
Copy link
Contributor

@pavinan could you cherry-pick this commit to target release/3.1? If that's difficult let me know, and we could do it on your behalf.

@pavinan
Copy link
Contributor Author

pavinan commented Oct 16, 2019

@pranavkm I don't know how to do cherry-pick in GitHub. You can do it.
Is it like another PR with base branch release/3.1?

@mkArtakMSFT mkArtakMSFT merged commit d73dfd3 into dotnet:master Oct 16, 2019
@mkArtakMSFT
Copy link
Contributor

Thanks for your contribution, @pavinan.

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

@gfoidl gfoidl gfoidl left review comments

@javiercn javiercn javiercn left review comments

@superdevguy superdevguy superdevguy approved these changes

Assignees
No one assigned
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@pavinan @rynowak @javiercn @pranavkm @mkArtakMSFT @gfoidl @superdevguy @Pilchie