CodeIt.Right

This forum is only for questions or discussions about working with the mojoPortal source code in Visual Studio, obtaining the source code from the repository, developing custom features, etc. If your question is not along these lines this is not the right forum. Please try to post your question in the appropriate forum.

Please do not post questions about design, CSS, or skinning here. Use the Help With Skins Forum for those questions.

This forum is for discussing mojoPortal development

This forum is only for questions or discussions about working with the mojoPortal source code in Visual Studio, obtaining the source code from the repository, developing custom features, etc. If your question is not along these lines this is not the right forum. Please try to post your question in the appropriate forum.

You can monitor commits to the repository from this page. We also recommend developers to subscribe to email notifications in the developer forum as occasionally important things are announced.

Before posting questions here you might want to review the developer documentation.

Do not post questions about design, CSS, or skinning here. Use the Help With Skins Forum for those questions.
This thread is closed to new posts. You must sign in to post in the forums.
11/12/2007 7:46:16 AM
Gravatar
Total Posts 46

CodeIt.Right

Hi Joe,
I noticed on the SVN checkin of your sandbox that you're playing around
with CodeIt.Right. I'd never heard of it before but it looks a pretty
neat tool. Are you planning on creating your own set of validation rules
for mojoPortal? I'd be interested in taking a look at them and seeing
what violations are thrown up in the survey feature if you are? Also I'd
be interested to know your thoughts on this tool and how you're hoping
to utilise it on mojoPortal?
Cheers,
Rob

11/12/2007 8:34:24 AM
Gravatar
Total Posts 18439

Re: CodeIt.Right

Hi Rob,

Yeah just learned about CodeIt.Right last friday and am using it to bring mojoPortal into conformance with the standard set of fxcop rules.

When I started friday there were 7721 violations and at the moment I'm down to 1619 (in the core features) and am hoping to get it down to 0 either today or tomorrow.

Part of the reason that I'm trying to get it all done at once is because changing names of things in an API can break other developers custom modules so those who have their own custom features are going to need to change their code after this. I don't think it will be difficult to fix using some find and replace, but I think making changes to the API need to happen pretty much all at once so that we don't break things repeatedly over time. This may be an aggravation to some and I may take some heat for making these changes but I think the long term benefit of coming into compliance with these rules is worth it because once we achieve compliance it will be much easier to stay in compliance as we go forward and this improves the quality of the code. I also plan to revise my Codesmith templates when I finish this cleanup so that we generate compliant code.

About a year and a half ago I tried to use FXCop but because it found so many violations it was looking like a huge daunting task to get into compliance and it was hard to try and be compliant even going forward because the new violations were in the same huge list as old ones. So CodeIt.Right seemed to provide an opprotunity to reduce this task to a doable level though I've got almost 3 full days into the cleanup as of now I figure even if it takes 2 more it will be worth it.

Most of the issues that developers will face with their custom features will be things that have changed the name to comply with the rules, so for example siteSettings.SiteID becomes siteSettings.SiteId so a find and replace of .SiteID with .SiteId will fix all of that particular one.

I've already had to fix some in the Survey feature and then I saw you made some commits to the survey feature so I figured once I cleanup the rest of the core I will get your latest code for the survey and fix it again to make sure I don't lose any of your work. The changes I make int he first round will be just to fix where I've broken things in Survey with my changes in core. After that is done and you have it synced to your sandbox, you are welcome to help fix violations within Survey. That would be great.

So I'm hurrying to finish this so that if there is going to be some pain for other developers its relatively all at once instead of strung out over time. I hope most developers understand the benefit and will be willing to bite the bullet and make the needed changes in their own code without getting too frustrated with me. I think already some of the breaking changes have made it to trunk so I'm trying to get the resto f them done quickly so they can all be dealt with at once. I would say that of the thousands of changes to come into compliance the vast majority are not breaking changes and have no impact on custom features but things in the SiteModuleControl base class, SiteSettings, PageSettings SiteUtils etc do potantially and probably impact custom features.

Best,

Joe

11/12/2007 12:58:22 PM
Gravatar
Total Posts 46

Re: CodeIt.Right

Sounds good. I'm sure the majority of people will understand. It's so much better working on code that has been enforced with a specific set of rules. I know what you mean about FXCop. I tried to run it over a few old work projects that were a lot smaller than Mojo but it still seemed like such a massive task to get everything implemented it was left. I'm happy to bring everything into line with the survey feature. I had a quick look at SurveyFeature.UI and it didn't throw too many violations so I'll try to bring everything into line over the next couple of weeks. One question, when you've brought everything into line will you use CodeIt.Right or FxCop to validate your new code?

Cheers for the advice,

Rob

 

11/15/2007 11:59:00 AM
Gravatar
Total Posts 46

Re: CodeIt.Right (Violation)

Hi Joe, I'm just sorting out the FXCop violations in SurveyFeature.UI and I have sorted all of them except the below: Type names should not match namespaces Violation Description The name of type WebStore.UI.layout matches the name of namespace layout in a case-insensitive comparison. Correcting Violation The rule provides the following auto-correct options: Rename type WebStore.UI.layout and its references to the specified name. I checked your sandbox and see you have the same naming still. What are your thoughts on this one? Cheers, Rob
11/15/2007 12:42:05 PM
Gravatar
Total Posts 18439

Re: CodeIt.Right

I think we're going to have to live with that one as fixing it will break every custom skin in the wild. You might be able to fix it in survey without breaking anything since the one in survey is only used to facilitate the build, its not actually used at runtime but the main one (in mojoPortal.Web) will definitly be a problem to fix and the cure may be worse than the sickness so I'm holding off on that one.

Best,

Joe

11/15/2007 3:12:58 PM
Gravatar
Total Posts 46

Re: CodeIt.Right (Another Question)

I guessed that would be the case. I’ll leave survey as-is to avoid confusion.


On another note I think I must have misunderstood about CodeIt.Right. I was under the impression that it would alert all FxCop rules? I fixed the 15 or so things in the SurveyFeature.UI that it flagged, but when I run Code Analysis in VS2005 which I believe uses the FxCop rules, it still flags 129 warnings that aren't shown in CodeIt.Right. So are you running against FxCop as well or am I missing something?
Cheers,
Rob

 

11/15/2007 3:13:28 PM
Gravatar
Total Posts 46

Re: CodeIt.Right (Another Question)

I guessed that would be the case. I’ll leave survey as-is to avoid confusion.


On another note I think I must have misunderstood about CodeIt.Right. I was under the impression that it would alert all FxCop rules? I fixed the 15 or so things in the SurveyFeature.UI that it flagged, but when I run Code Analysis in VS2005 which I believe uses the FxCop rules, it still flags 129 warnings that aren't shown in CodeIt.Right. So are you running against FxCop as well or am I missing something?
Cheers,
Rob

 

11/15/2007 4:56:30 PM
Gravatar
Total Posts 18439

Re: CodeIt.Right

Well, yes I plan to follow up with FxCop but for now I'm still plowing through the lower hanging fruit that CodeIt.Right finds. Actually there is not much low hanging fruit left and the remaining erors are taking longer and longer to fix. Some things I don't dare chage unless I'm ready to do a lot of testing and other things I'm not sure I'm going to change even if it means I can't get to 0 FxCop violations. For example they don't like my mojoPortal namespace or any classes that start with mojo, they want it to be Mojo but I'm not all that inclined to go along with it. Also these tools find things that are just not problems like if you have method GetSPParameters(..) it suggests renaming to GetSpParameters but after you do it it suggests changing it back! So some human judgement still must prevail.

Cheers,

Joe

11/17/2007 7:48:35 AM
Gravatar
Total Posts 46

Re: CodeIt.Right (Another FxCop violation question)

Hi Joe,
Can you offer me some advice on the following FxCop violation? I’ve seen a few recommendations how to deal with this but I’m keen not to make changes that cause culture specific problems in MojoPortal.

Type Name: SpecifyIFormatProvider
A method or constructor calls one or more members that have overloads that accept a System.IFormatProvider parameter, and the method or constructor does not call the overload that takes the IFormatProvider parameter. This rule ignores calls to .NET Framework methods that are documented as ignoring the IFormatProvider parameter and additionally the following methods:

Cheers,
Rob

11/17/2007 8:07:24 AM
Gravatar
Total Posts 18439

Re: CodeIt.Right

Hi Rob,

I think those are usually associated with code that uses .ToString()

I'm not usre the best way for all data types but for int variables I think the way to resolve it is to use

using System.Globalization

.ToString(CultureInfo.InvariantCulture)

InvariantCulture tells it not to use the current culture but to toString it in a culture neatral way which is virtaully always what we want with ints and probably other data types too unless there is a specific reason to format it to the current culture. I guess if its being displayed in the ui you might use the current culture in some cases like .ToString(CultureInfo.CurrentCulture)

Best,

Joe

ps I ran FxCop and there are way more violations than what CodeIt.Right found. Kind of disheartenting. I'm burnt out from doing this grunt work for a week and need to move on something more rewarding for now but plan to do more and more over time in the late part of the day when my mind is less creative and gradually continue to fix these.

11/17/2007 8:32:55 AM
Gravatar
Total Posts 46

Re: CodeIt.Right

That makes sense. I was surprised when I saw how many extra violations that FxCop throws. It’s still great progress to have the CodeIt.Right violations fixed though!

You must sign in to post in the forums. This thread is closed to new posts.