-
Notifications
You must be signed in to change notification settings - Fork 21
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
Call correct cache clearing API when moving topic #120
Conversation
Great, thanks again for spending time on this :-) |
(Should I post this in discussions?)
What is the conventional wisdom on general refactoring and/or language features? For example, interpolated strings were introduced in c# 6.0. Since DNN requires net framework 4.7+, that implies support for c# 6.0.
Same for auto properties in c# 3(?). They make a HUGE improvement in code readability and maintenance.
Not saying go make sweeping changes but if you’re in the code, do some useful refactoring.
Or is it a case of, if it ain’t broke just leave it alone?
John
…________________________________
From: Timo Breumelhof ***@***.***>
Sent: Thursday, February 10, 2022 11:33:00 AM
To: DNNCommunity/Dnn.CommunityForums ***@***.***>
Cc: John Henley ***@***.***>; Author ***@***.***>
Subject: Re: [DNNCommunity/Dnn.CommunityForums] Call correct cache clearing API when moving topic (PR #120)
Great, thanks again for spending time on this :-)
—
Reply to this email directly, view it on GitHub<#120 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACI4JZXRUPEJDPAZMHPPFNTU2PSDZANCNFSM5OBHBBHQ>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
@johnhenley IMO refactoring is always good if it's not too much work. |
I agree with the advice given by @Timo-Breumelhof. In general, refactoring is good, but we should do it carefully and in planned phases. We shouldn't do it as part of a different update, in my opinion. For example, an issue and then PR that simply updates interpolation strings would be great. Then, a separate issue and PR can be used for the next optimization. If something minor is found within a specific update that's related, that would be different and fine when included. For example, creating an optimization within the current class file that has other updates can be fine - but it depends on the risk of the specific update in general. This would be determined on a case-by-case basis. It's not always clear where the line is. We just need to continue to chat about it like we are now, whenever we all want to figure something like this out. |
In this update, specifically... Where did you come up with the cache token? |
Did you intend to close and delete this PR? |
@WillStrohl Oops. Another GitHub rookie mistake I guess. I renamed my local branches (I changed from bug/# and task/# to issue/# so they line up better in visual studio ...) and I guess that screwed up the linkage to remote. |
We'll eventually want to turn a format value like that into a constant, but this is fine for now. Thanks for your contribution! |
|
Indeed. Too many. 😁 |
Call correct cache clearing API when moving topic
Changes made
PR Template Checklist
Please mark which issue is solved
Close #65