Giter Site home page Giter Site logo

Comments (36)

 avatar commented on May 5, 2024

I like the use Var whenever possible. But I see why it isn't good in all cases, a good compromise is use Var when the initalizer has specified the base type that way you know what the type is immediately. Something which R# can do.

Underscores, ALLCAPS and the stuff in the Native namespace needs to be renamed to be stylecop compliant.

For all of my projects I use R# + Stylecop and strive for 100% stylecop compliance. I say let's just go full stylecop compliance, which would be the easiest and most accepted style. Currently my fork is getting there but it's missing proper file headers which I could easily generate.

from mahapps.metro.

DoCode avatar DoCode commented on May 5, 2024

code without regions
better small and clean files, classes, methods >>> then nothing needs to be unfolded

from mahapps.metro.

flagbug avatar flagbug commented on May 5, 2024

I personally use var only when the type is obvious, for example

var foo = new Bar();

but if it's not obvious, I use

Bar foo = GetFromSomewhere();

from mahapps.metro.

shiftkey avatar shiftkey commented on May 5, 2024

Bar foo = GetFromSomewhere();

Without more context, this indicates to me that the method name isn't clear.

from mahapps.metro.

 avatar commented on May 5, 2024

That's what the compromise could be. R# has a setting that allows that.

As for regions: It is possible to configure the Visual Studio IDE to not fold any of the regions when files are opened, but this is the out of box behavior.

I'm impartial to regions. I think they are nice to have you use them, they don't do anything if you don't.

from mahapps.metro.

DoCode avatar DoCode commented on May 5, 2024

who needs to regions on a particular focus put on, that has classes too large / methods

from mahapps.metro.

 avatar commented on May 5, 2024

I'm against regions regardless of what the settings of VS are.

from mahapps.metro.

shiftkey avatar shiftkey commented on May 5, 2024

On regions: this is my manifesto - http://www.richard-banks.org/2011/02/anti-region-campaign.html

There is one exception to this - Dependency Properties - where I will accept #region usage.

from mahapps.metro.

 avatar commented on May 5, 2024

While I understand that dependency properties are ugly and shameful, in a UI based library, hiding UI code is a pain.

Structure them better so that they're more logical, rather than sweeping them under the region rug.

from mahapps.metro.

thoemmi avatar thoemmi commented on May 5, 2024

By

Underscores for field names - no. An icky reminder of times past.

I guess you refer to private fields?

What about the bracing style? I prefer K&R, but you seem to code in Allman style.

How about committing ReSharper's .DotSettings file to the repository?

from mahapps.metro.

 avatar commented on May 5, 2024

I just commited by R# settings to my fork.

from mahapps.metro.

shiftkey avatar shiftkey commented on May 5, 2024

@sevenalive I don't see it here: https://github.com/sevenalive/MahApps.Metro/commit/639a3d07a76c1575fdc73c8eb4fb5c1d75475877

from mahapps.metro.

 avatar commented on May 5, 2024

https://github.com/sevenalive/MahApps.Metro/commit/d0466bb469dda91eb13070d9aa47fc77e71f256e

from mahapps.metro.

shiftkey avatar shiftkey commented on May 5, 2024

New rule: No code headers on files for OSS projects.

  • distracts from the actual code
  • often do not match the project's licence
  • there are other ways to view those details - GitHub tracks contributors to a file without any additional effort.

from mahapps.metro.

 avatar commented on May 5, 2024

I'm against this one. File headers are a good thing.

How about removing the author info and instead of having the license in the file, just put it as a link.

So the header keeps the project copyright and license.

I fail to see how this gives code readability problems.

from mahapps.metro.

shiftkey avatar shiftkey commented on May 5, 2024

The project copyright is set at the project level - not the code file level. A file at the root of the repository is all that you should require. Can you give me a compelling reason why you want to mix and match them (or duplicate this across all the code files)?

Having the license in each file also doesn't cover the possible file types. What about XAML files - should the also include it?

from mahapps.metro.

shiftkey avatar shiftkey commented on May 5, 2024

Alternatively, can you share what you consider to be an unobtrusive code file header? I'm aware that GPL licenses and their kin recommend this approach, but I'm trying to approach this from an agnostic view.

from mahapps.metro.

 avatar commented on May 5, 2024

Most licenses say you can't remove copyright information. So if a file has a copyright notice in the file like most files it would be a violation of that.

Plus let's say I want to use the progress indicator but don't want anything else. I would just copy the files I needed and add to my own library.

So now the code doesn't have a header.
So now if someone browses my repo, they will think I made it and it's under my license.

from mahapps.metro.

shiftkey avatar shiftkey commented on May 5, 2024

Plus let's say I want to use the progress indicator but don't want anything else. I would just copy the files I needed and add to my own library.

Where I've done things like this in the past (used code from another source/project) I will attribute it in the header.

We do this already in MA.M https://github.com/MahApps/MahApps.Metro/blob/master/MahApps.Metro/Controls/MetroContentControl.cs

The consumer may just be as likely to remove the header and ignore whatever license you set. This may be a concern for many but for our scenarios - where we want to share code under a permissive license - we are not interested in spending time chasing down violations.

from mahapps.metro.

 avatar commented on May 5, 2024

Could do something like:
$USER_LOGIN$

from mahapps.metro.

shiftkey avatar shiftkey commented on May 5, 2024

I've started a very simple R# settings file here.

The purpose of this settings file is to make the codebase more consistent over time, rather than introducing broad sweeping changes unnecessarily.

I'm getting @Aeoth to road test it - we can refine it over time as people add in new features and bugfixes.

from mahapps.metro.

 avatar commented on May 5, 2024

Without stylecop installed R# is not fully compliant with it. So did you reset R# to follow stylecop then make your changes to it at that point?

Why are you keeping the ALLCAPS for the win32 api stuff. I rather have them renamed to follow c# naming conventions.

Right now the codebase is unorganized. So I think in the very near future there should be a code cleanup. Let's schedule that for next week so everything can push their changes before the cleanup.

from mahapps.metro.

 avatar commented on May 5, 2024

I don't care about StyleCop compliance.

from mahapps.metro.

 avatar commented on May 5, 2024

@shiftkey As I said, send a pull request if you want me to check things out.

from mahapps.metro.

 avatar commented on May 5, 2024

Right now the codebase is unorganized.

@sevenalive What particular parts aren't organised? Is it that they don't have stylecop organisation that you're complaining about? If there is something genuinely difficult to follow rather than styling issues, I'm happy to have a cleanup, but not for the sake of it doesn't "comply" with your views.

from mahapps.metro.

shiftkey avatar shiftkey commented on May 5, 2024

@sevenalive StyleCop is not an immediate concern for me. Baby steps.

I've kept those win32 API classes as is to follow the win32 naming style for structs and parameters.

I would prefer to get the rules in place, and then when someone modifies a class they have a secondary commit to cleanup the class (if they want).

from mahapps.metro.

 avatar commented on May 5, 2024

Everything is out of order and not sorted. You have private methods before public methods. Some class files have dependency properties before the constructor, the next file will have them after the constructor.

That does hurt readability. Nothing is standardized right now.

What I am saying is start with R# stylecop default rules, let's spend a week proposing any changes to those rules. Then at the end of the week you run R# cleanup. Out of the box without stylecop installed, R# rules are not compliant. I went ahead and stripped out all the unneeded settings and added a solution r# settings file in my fork.

I know that stylecop isn't the end all to style issues but it's a great starting point. First off we can ignore the file header rules that require company tag and even the copyright tag.

That way people who have uncommitted changes can push those changes before the cleanup and then everything will be standardized going forward. Let's do that now while the project contributors are small.

As for file headers, most licenses state you can't remove any copyright info. So I think a simple 2 line file header would do the trick.
// MahApps.Metro Project
//

That way If someone takes just 1 file it keeps the copyright information and those less lazy to attribute work don't need to do anything.

from mahapps.metro.

 avatar commented on May 5, 2024

No fileheaders will be accepted.

from mahapps.metro.

shiftkey avatar shiftkey commented on May 5, 2024

You have private methods before public methods.

This is a StyleCop rule.

Out of the box without stylecop installed, R# rules are not compliant.

Compliant with what?

from mahapps.metro.

 avatar commented on May 5, 2024

You are incorrect with that. Public goes before private. Properties go before methods. fields are before constructors

Also the code is not wrapped to 120cpl, not wrapping code hurts readability and productivity.

from mahapps.metro.

 avatar commented on May 5, 2024

I've got a nice widescreen monitor, turn on word wrapping if thats an issue for you.

This discussion is going around in a circle. Ultimately I'm not going to change my project for those who have blind adherence to tooling. For example, looking at SevenUpdate, your XML comments in App.xaml.cs frankly scare the crap out of me.

I'm willing to host and adhere to other code style changes, but not ones that get in the way of me maintaining and developing my project.

from mahapps.metro.

 avatar commented on May 5, 2024

How exactly is code documentation bad, especially in a OSS project? Code
readability and documentation should be the first 2 priorities. The XML
documentation provides 2 things, it tells someone who isn't me what my code
does and it provides intellisense support. Whether it's open source or not,
you aren't writing code for yourself, you are writing code for everyone.

It seems that you don't really want anyone else to contribute to "your"
open source project. Standards exist for a reason, it's not just you and
your friend who works on the code. I'm just asking for the code to be
standardized by at least some standard. When I browse your code you have
properties at the end of the file after methods, in another file they are
before the methods. The code is all over the place.s

I have a widescreen monitor too and for a while I didn't wrap my code
either but there are very good reasons to wrap your code. If code is
wrapped, you can have 2 code files open side by side and see everything,
no horizontal scrolling. Having visual studio do the wrapping at the IDE
level does not solve any of these problems.

Plus you can't even view all of your code on github, most code hosting
sites set their display width to 120 because that is a widely accepted
standard.

http://www.cookcomputing.com/blog/archives/000545.html

I have made compromises. I removed regions because I realize there is no
real value with them, I shorted the file headers down to
only relevant information. Almost every major open source project includes
file headers, like wordpress, android, and many other projects.

from mahapps.metro.

shiftkey avatar shiftkey commented on May 5, 2024

Its a bit unfair to cite major open source projects which have paid employees to support them and hold @Aeoth to the same expectations.

And please read this: http://tirania.org/blog/archive/2010/Dec-31.html

from mahapps.metro.

shiftkey avatar shiftkey commented on May 5, 2024

I'm just asking for the code to be standardized by at least some standard. When I browse your code you have
properties at the end of the file after methods, in another file they are before the methods.

As an aside - if this had been raised initially as a specific issue, the discussion may have gone differently. That's something much easier to discuss and agree on than coding standards as a whole.

from mahapps.metro.

 avatar commented on May 5, 2024

@sevenalive I've accepted plenty of contributions to MA.M, because other people haven't demanded that I change everything to their way of coding when they're asking me to pull in code.

Your attitude is doing nothing to help your cause, please don't bother submitting any more code.

from mahapps.metro.

 avatar commented on May 5, 2024

I did no such thing, yes the first PR did have cleanup, but I redid the PR and you still denied it because of the Resharper settings files that you asked me to commit. So I then removed the personal stuff from the file and you still denied it. You could have accepted the PR and deleted the R# settings file if that was such a problem.

Nothing in that pull request refactored the code, except for putting the PercentageConverter class it's in own file and moving it to the converter folder where the rest of the converters are.

I thought we were deciding on what to do about the code style and having a discussion about it. You then just said you weren't going to change anything and closed the issue your partner created.

You said no file headers and no code wrapping, I simply provided valid arguments for it. The only thing I request is standardize the code now instead of doing it only when a file is edited. Putting it off doesn't solve anything and just introduces PR issues and inconsistency.

I think code wrapping should be done, you can't view the code without a lot of horizontal scrolling on github and not everyone runs the same resolution. I run 1080P so unwrapped code is fine for most of the codebase except the really long xaml.

What about the people who run lower resolutions or viewing the code on github? Without wrapping you are really just wrapping to the resolution of your monitor as that article I linked explains. It also makes it harder to read.

Sure I could turn on word wrap at the IDE level, but that contradicts your own argument you used for Regions. You can turn off region hiding at the IDE level but you didn't want to do that. So I find that ironic.

So it just comes down to consistency. Shiftkey asked for a discussion on it and that is what we are doing. I am asking you to consider the feedback instead of just saying no.

from mahapps.metro.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.