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

V8: Updated Constant.System.Root references (to use correct constant, and others) #5109

Merged
merged 6 commits into from
Apr 1, 2019

Conversation

leekelleher
Copy link
Member

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes

Description

I'd noticed there were several Constants.System.Root.ToInvariantString() calls, (e.g. to convert the int to a string), but there is an existing Constants.System.RootString constant (string). So I've swapped them over.

This should reduce the number of string conversions / new allocations. A teeny-weeny CPU/memory improvement. 😉

I also did the Constants.System.RecycleBinContent and Constants.System.RecycleBinMedia references too.

I've re-ran all the unit-tests in Visual Studio, all passed - and checked the CMS back-office, navigated around the content section.

I wouldn't say that I've done extensive testing against an existing/populated Umbraco instance, since this felt like a low-risk code change.

to use the `System.RootString` constant.

This saves on integer parsing and reduces the number of
new strings being created.

Since the `System.Root` is `-1`, it didn't need the culture invariant
conversion.
to use the `System.RecycleBinContentString` constant.

This saves on integer parsing and reduces the number of
new strings being created.

Since the `System.RecycleBinContentString` is `-20`, it didn't
need the culture invariant conversion.
to use the `System.RecycleBinMediaString` constant.

This saves on integer parsing and reduces the number of
new strings being created.

Since the `System.RecycleBinMedia` is `-21`, it didn't
need the culture invariant conversion.
Also removed unused namespaces.
@ghost ghost assigned leekelleher Mar 29, 2019
@leekelleher leekelleher changed the title Updated Constant.System.Root references (to use correct constant, and others) V8: Updated Constant.System.Root references (to use correct constant, and others) Mar 29, 2019
@leekelleher leekelleher removed their assignment Mar 29, 2019
@zpqrtbnk zpqrtbnk changed the base branch from dev-v8 to v8/dev March 30, 2019 10:25
@Shazwazza
Copy link
Contributor

@warrenbuckley if you get a chance could you have a quick look? Looks like a great and easy change. If/when you merge please flag for 8.1 release

@warrenbuckley
Copy link
Contributor

Code changes look fine. Azure DevOps built this against V7 for some reason.
I am just waiting for tests to pass with it targeting V8 build then should be all good :)

https://umbraco.visualstudio.com/Umbraco%20Cms/_build/results?buildId=19632

@warrenbuckley
Copy link
Contributor

warrenbuckley commented Apr 1, 2019

All passed & code reads fine - merged in.
Thanks @leekelleher 💪

@warrenbuckley warrenbuckley merged commit 3af8d0e into umbraco:v8/dev Apr 1, 2019
@leekelleher leekelleher deleted the chore/constants-root branch April 1, 2019 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants