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

[doc] Metrics documentation #517

Merged
merged 4 commits into from Jul 28, 2017
Merged

[doc] Metrics documentation #517

merged 4 commits into from Jul 28, 2017

Conversation

oowekyala
Copy link
Member

A starting point for the documentation of metrics

@jsotuyod jsotuyod added this to the 6.0.0 milestone Jul 25, 2017
@jsotuyod jsotuyod added the in:documentation Affects the documentation label Jul 25, 2017
@adangel adangel added the in:metrics Affects the metrics framework label Jul 25, 2017
Copy link
Member

@adangel adangel left a comment

Choose a reason for hiding this comment

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

Great, that's a good start! Just rename the files, and I'll merge then.

to extend the framework with their own custom metrics."
last_updated: July 20, 2017
sidebar: pmd_sidebar
permalink: pmd_devdocs_metrics_howto.html
Copy link
Member

Choose a reason for hiding this comment

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

I've meanwhile changed my naming convention: since we have "pmd" and "devdocs" already in the path as folder, I decided to do not repeat these in the filename. So, please rename this file to just metrics_howto.md.
Important: Keep the "permalink" as it is - this will be the final name of the html file, that is generated. Since this jekyll theme generates all pages flat in the root directory, it's easier to see then where this page belongs to (and avoid any clashes).

Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering, whether we really need this flat structure or whether we could reuse the folder structure? One argument was, that it's easier, if you link between the pages or include images, etc. You do not need to care about relative paths... But maybe, it would be better to have urls like https://pmd.github.io/pmd/devdocs/metrics_howto.html instead of https://pmd.github.io/pmd/pmd_devdocs_metrics_howto.html?

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay I'll rename it.

I think it would be nice to have a proper path in urls, it's more structured and overall cleaner. I fiddled a bit with jekyll but found no way to do that... The documentation theme's doc seems to hint that this is not possible:

The listing of all pages in the root directory (which happens when you add a permalink property to the pages) is what allows the relative linking and offline viewing of the site to work.

Besides the sidebar seems to remove any forward slash it finds in urls, so we cannot even specify urls manually. I guess the theme doesn't give us the choice


### Versions

* Version `CycloVersion#IGNORE_BOOLEAN_PATHS`: Boolean operators are not counted, nor are empty
Copy link
Member

Choose a reason for hiding this comment

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

Can we give here a reason, why we have this variant? Was it because of backwards-compatibility?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right this can be more detailed. It was to provide results similar to what the old StdCyclomaticComplexity provided, but as the rule will be removed I guess it's not even for backwards compatibility

variables declared on the same line (e.g. `int a, b, c;`) as a single statement.
* Contrary to Sonarqube, but as JavaNCSS, we count type declarations (class, interface, enum, annotation),
and method and field declarations \[[Sonarqube](#Sonarqube)\].
* Contrary to JavaNCSS, but as Sonarqube, we do not count package declaration and import declarations as statements.
Copy link
Member

Choose a reason for hiding this comment

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

I like to have the reasoning behind this... If I remember correctly, ignoring package/import declarations makes it easier, to compare two classes and compare a inner class with a top-level class, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, at least in theory. Using this definition, the Ncss count of a class is only the statements that are inside the declaration (plus the declaration statement), which I think makes extra sense since import statements are more metadata for the compiler than code. Plus, most IDEs fold them by default so they don't hamper readability nearly as much as eg many field declarations

@adangel
Copy link
Member

adangel commented Jul 28, 2017

I'll merge this, although we'll probably change the location/structure, where we place the list of available metrics. We were discussing somewhere under language specific documentation, if I remember correctly. We should probably move then the rule documentation (see #526) also under the language specific section.

adangel added a commit to adangel/pmd that referenced this pull request Jul 28, 2017
@adangel adangel merged commit 97b3a59 into pmd:master Jul 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in:documentation Affects the documentation in:metrics Affects the metrics framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants