Move everything under package dscanner

[LaurentTreguier]

This is the same as dlang-community/DCD#417. Since #469, D-Scanner can be used as a library, and it would make much sense to import dscanner.foo instead of just import foo.

[WebFreak001]

nonono there isn't even a working release on dub yet, 0.4.0 still had some issues if you tried to use that as a library (lots of linker errors because submodules weren't included). Doing this will break everything using dscanner as a library because you need to do ~master.

Please DO NOT MERGE until there had been a release for a few weeks

[LaurentTreguier]

Well I am not supposed to even be able to merge this, if you request a review it should prevent the dlang-bot from doing anything (if this is how it works)

EDIT : or should I close this PR to stall it until you think it's ready ?

[WebFreak001]

There needs to be a new DScanner release first because the current latest tag is broken as a library. Once the release was out for a few days or weeks this can be merged, but before that it shouldn't be merged.

[WebFreak001]

I can mark a 0.5.0 release as there was no change to src/ for 3 months, @Hackerpilot @bbasile what would you say?

[Hackerpilot]

That's fine so long as we move all the uncompleted issues out of https://github.com/dlang-community/D-Scanner/milestone/6 first.

[ghost]

The two remaining items in the 0.4.1 milestone should be easily fixable. Unless you're in a hurry i don't see the point of procrastinate.

[WebFreak001]

yeah I think it's a good idea to finish 0.4.1 and release that

[ghost]

Oops, assuming the config based on the makefile passes the tests of course.

[LaurentTreguier]

@bbasile for some obscure reason, moving everything under the dscanner package makes dscanner ignore the config file and thus it begins spraying warnings all over the place... Now that's really weird.

[ghost]

add an issue make dscanner a package to milestone 0.4.1 and then people start to fight so that this PR passes the tests. I cant do more, i know i was mentioned in this topic but actually i've left the organization.

[ghost]

This PR doesn't seem to be maintained. I'll close on 7 days if nothing done before.

[LaurentTreguier]

Whoops. This issue had completely slipped out of my mind... I've rebased my fork on master, it compiles and passes the tests Linux and Windows on my end.

[ghost]

self linting must be repaired, the following check fails :

$ ./bin/dscanner --config .dscanner.ini --styleCheck src

The ini file doesn't seem to be applied anymore.

[ghost]

or bin/dscanner

[LaurentTreguier]

Or even /bin entirely ? That would also exclude unittest binaries

[ghost]

yeah

[LaurentTreguier]

Oh, I found out why .dscanner.ini is not respected anymore. The INI header is [analysis.config.StaticAnalysisConfig] based on the fully qualified name of the static analysis class because of inifiled... So changing it to [dscanner.analysis.config.StaticAnalysisConfig] in .dscanner.ini fixes the issue; however this means that any existant dscanner.ini file would need to be patched...

[ghost]

D allows to have a path and a different module name. If you keep the old module name it will work. You just have a few imports statements to revert.

[ghost]

That being said this is not clean and would have to be changed for the next minor version anyway.
What do you think @Hackerpilot, @wilzbach, @WebFreak001 ?

[LaurentTreguier]

Hackerpilot had directed me to the specs (dlang-community/DCD#417 (comment)), and they say that packages should correspond to the filesystem path (https://dlang.org/spec/module.html#module_declaration).
The aim of this PR was to facilitate the usage of D-Scanner as a library, so keeping an old package would be still somewhat quirky

[ghost]

Changing the file without user agreement is not something to do so the only other options are breaking existing config files or not changing the module names.

[LaurentTreguier]

Yes, by patching the dscanner.ini files I meant it as a manual patch from the user; since although it's technically a breaking change, it's about changing one line in a config file

[ghost]

At least change the ini used by this repo so that the tests become green.

[WebFreak001]

hm not a fan of requiring to change all configs... Can't the ini parser do it just with the struct/class name if it knows about the possible type already?

[LaurentTreguier]

You mean, automatically trying to get the config section even if the name of the section is not the fully qualified name of the class/struct ?

[ghost]

@WebFreak001, it would not be the first time the INI format breaks. I did this before for the switch allowing to skip the unittest.

[WebFreak001]

I guess we aren't 1.0.0 yet, but maybe the ini format should be changed in a separate PR so it allows non fully qualified names in the future.

[WebFreak001]

I think the windows test there segfaults, so you would need to fix that but other than that I'm fine with this change now

[LaurentTreguier]

The Windows x64 tests already segfault even without my changes as far as I know (I'll double check that when I can)

[ghost]

@LaurentTreguier: sorry there are again conflicts due to other PR merged.

[LaurentTreguier]

@bbasile whatever it is it should't be hard to rebase again. I can also confirm that D-scanner is crashing on Windows when compiling for x64 and trying to test it. Visual Studio's just-in-time debgger is telling me that the problem comes from libdoc, in src/sections.d, line 158:

I have no idea why this happens.

[ghost]

i'm gonna tag to 0.5.0-beta.1, after this PR merged, leaving the chance to a Windows user to fix x64 but not for too long, see milestone.