Fix CircleCi style checker by wilzbach · Pull Request #4850 · dlang/phobos

wilzbach · 2016-10-11T10:03:38Z

It seems like we/I accidentally disabled the style target on CircleCi (style is not defined in the command switch). I take the full blame for this, but at least it's a good proof that the CI actually does catch quite a few things ;-)

I split up the fixes depending on their category.

There's only one line left about which DScanner is complaining (hence the "blocked" label):

public static immutable ctRegex(alias pattern, alias flags=[]) = ctRegexImpl!(pattern, flags).nr;

It warns with:

std/regex/package.d(421:32)[error]: Declaration expected
std/regex/package.d(421:62)[error]: Declaration expected
std/regex/package.d(421:64)[error]: Declaration expected
std/regex/package.d(421:97)[warn]: Empty declaration

@Hackerpilot is there a simple fix? Otherwise I will add this file to the Dscanner blacklist.

Hackerpilot · 2016-10-11T19:45:49Z

I'll have to figure out what kind of declaration that is and then update the parser.

Hackerpilot · 2016-10-13T02:34:25Z

This should help: https://github.com/Hackerpilot/Dscanner/releases/tag/v0.4.0-beta.1

Hackerpilot · 2016-10-14T10:26:10Z

Interesting. The compiler used by Travis can't find the mallocator module. Why is it using dmd-2.068.2?

MartinNowak · 2016-10-19T10:39:56Z

Why is it using dmd-2.068.2?

Because you're not specifying with which dmd to build dscanner. It just picks up whatever system-wide dmd installed. Please fix that, e.g. similarly to how we pin stable dmd versions for building dlang.org's ddocs or dmd's bootstrap compiler.

wilzbach · 2016-10-21T14:12:08Z

... and now we run into problem that currently our installer is broken for point releases :/
(See dlang/installer#201 for more details)

wilzbach · 2016-11-22T20:59:54Z

Okay I once again updated this PR to include the changes that managed to sneak in over the last month.
I also had to disable Dscanner's new check for auto return functions which return void as it's commonly used like version(Stddoc) auto myfunction(){ assert(0)} or with mixin.

I hope that this version now passes our CI, s.t. we can finally enable the check again :)

JackStouffer · 2016-12-06T12:51:10Z

std/random.d

 {
-    import std.algorithm : canFind, map;
+    import std.algorithm.searching : canFind;
+    import std.algorith.iteration : map;


Failing on this line. algorithm is misspelled

wilzbach · 2016-12-08T01:17:42Z

I went through another round of catching up & thus rebased to the lastest changes. It would be quite nice if we can get this in rather sooner than later :)

CyberShadow · 2016-12-08T05:35:07Z

std/experimental/ndslice/internal.d

    }

-    auto opBinary(string op)(sizediff_t shift)
+    typeof(this) opBinary(string op)(sizediff_t shift)


This is technically a false positive, right?

Yes unfortunately Dscanner can't expand mixins atm, s.t. is the poor man's workaround "for the greater good".

Oh it seems that Dscanner got even smarter: dlang-community/D-Scanner#379, so I reverted that workaround.

CyberShadow · 2016-12-08T05:37:21Z

Auto-merge toggled on

CyberShadow · 2016-12-08T05:37:56Z

How come I don't see CircleCI in the CI services list on this PR?

CyberShadow · 2016-12-08T05:38:04Z

Auto-merge toggled off

JackStouffer · 2016-12-08T10:23:20Z

Its actually disappeared from all PRs, so probably not due to these changes.

CyberShadow · 2016-12-08T10:37:25Z

Perhaps it's because of this setting?

Link goes to https://circleci.com/docs/fork-pr-builds/.

I don't know how CircleCI is configured (heck, I have no clue why we even need it at all considering there's Travis and AppVeyor), so I'm not sure if it's safe to enable or if it's relevant at all. @wilzbach ?

wilzbach · 2016-12-08T11:16:16Z

How come I don't see CircleCI in the CI services list on this PR?

Oh sorry - I was hoping that it was just a temporary outage and would start to build the PR.
-> I will have a look :(

I don't know how CircleCI is configured (heck, I have no clue why we even need it at all considering there's Travis and AppVeyor), so I'm not sure if it's safe to enable or if it's relevant at all. @wilzbach ?

The only reason for using CircleCi is because it's not Travis :P
With the free plan an organization (e.g. dlang) gets five concurrent slots and well with all it's parallel builds dmd uses all of them.

CyberShadow · 2016-12-08T11:17:59Z

The only reason for using CircleCi is because it's not Travis :P

So, to clarify... CircleCI has more throughput than Travis, or using both gets us more throughput than using just one, or something else?

wilzbach · 2016-12-08T11:27:14Z

So, to clarify... CircleCI has more throughput than Travis, or using both gets us more throughput than using just one, or something else?

Yes it's solely about having more throughput. We tried to use Travis for Phobos before and the builds were stuck in the queue for days, hence the move to CircleCi.
The use of CircleCi for code coverage testing at dmd and druntime has a similar motivation.

I don't know the reason for the outage with CircleCi yet, but unfortunately it doesn't have anything to do with the "Permissive build setting").
Anyways thanks to @MartinNowak the CI logic is in this separate circleci.sh, so we are pretty agnostic about the CI provider.

wilzbach · 2016-12-08T12:19:32Z

Perhaps it's because of this setting?

Seems like you were right. As we don't have any sensitive information stored with CircleCi, I flipped "Permissive building of fork pull requests" to "on" and it seems to be building now :)

While I was looking at our config I also realized that a bug in Dscanner has been fixed and we can now re-enable the style checking for std/conv.d. Sorry for the noise :/

CyberShadow · 2016-12-08T12:40:09Z

Auto-merge toggled on

wilzbach added the Merge:Blocked label Oct 11, 2016

wilzbach force-pushed the fix_circleci_style branch from 7459c15 to 6415fa3 Compare October 11, 2016 11:42

Hackerpilot mentioned this pull request Oct 12, 2016

Compile errors on DMD 2.072.0-b2 (breaks DDOX and DUB's test suite) dlang-community/libdparse#112

Closed

wilzbach force-pushed the fix_circleci_style branch 2 times, most recently from 96f7ee3 to 2f54a27 Compare November 22, 2016 20:58

wilzbach removed the Merge:Blocked label Nov 22, 2016

ghost mentioned this pull request Nov 26, 2016

auto functions checker, fix several cases of false warnings dlang-community/D-Scanner#379

Merged

JackStouffer reviewed Dec 6, 2016

View reviewed changes

wilzbach added 9 commits December 8, 2016 01:46

Style fix: add whitespace after if/while

df2e72d

Style fix: Add whitespace between import colon

c5ba7c7

Style fix: package wide std.algorithm imports

1ca4d31

Style fix: enforce allman style

6f1690f

Style fix: specify/remove local imports

64217c8

Fix CircleCi: correctly enable style target

813be1f

Update Dscanner to v0.4.0-beta.1

964c1e5

Fix DScanner version to a specific DMD release

5985eac

Follow-up style fixes for November

2c81663

wilzbach force-pushed the fix_circleci_style branch from 2f54a27 to 6a0e0b2 Compare December 8, 2016 00:56

This was referenced Dec 8, 2016

Issue 8573 - A simpler Phobos function that returns the index of the … #4921

Merged

fix issue_15421 [reloaded] - The behaviours of the topNs differ with the bottom #4928

Merged

wilzbach assigned andralex Dec 8, 2016

CyberShadow reviewed Dec 8, 2016

View reviewed changes

CyberShadow approved these changes Dec 8, 2016

View reviewed changes

wilzbach added 3 commits December 8, 2016 12:32

Use void for auto function without return statement

b82ae35

Disable check for auto functions without return statement

55898ea

Follow-up style fixes for December

f0c5a9f

wilzbach force-pushed the fix_circleci_style branch from 6a0e0b2 to f0c5a9f Compare December 8, 2016 11:33

Reenable style checking of std/conv.d

0e9f16e

wilzbach force-pushed the fix_circleci_style branch from ecfc617 to 0e9f16e Compare December 8, 2016 12:15

CyberShadow merged commit 8b2a338 into dlang:master Dec 8, 2016

wilzbach deleted the fix_circleci_style branch December 8, 2016 13:58

wilzbach mentioned this pull request Dec 23, 2016

CircleCi: merge PRs with upstream branch dlang/druntime#1723

Merged

Uh oh!

Comments

Conversation

wilzbach commented Oct 11, 2016

Uh oh!

Hackerpilot commented Oct 11, 2016

Uh oh!

Hackerpilot commented Oct 13, 2016

Uh oh!

Hackerpilot commented Oct 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MartinNowak commented Oct 19, 2016

Uh oh!

wilzbach commented Oct 21, 2016

Uh oh!

wilzbach commented Nov 22, 2016

Uh oh!

JackStouffer Dec 6, 2016

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Dec 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CyberShadow Dec 8, 2016

Choose a reason for hiding this comment

Uh oh!

wilzbach Dec 8, 2016

Choose a reason for hiding this comment

Uh oh!

wilzbach Dec 8, 2016

Choose a reason for hiding this comment

Uh oh!

CyberShadow commented Dec 8, 2016

Uh oh!

CyberShadow commented Dec 8, 2016

Uh oh!

CyberShadow commented Dec 8, 2016

Uh oh!

JackStouffer commented Dec 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CyberShadow commented Dec 8, 2016

Uh oh!

wilzbach commented Dec 8, 2016

Uh oh!

CyberShadow commented Dec 8, 2016

Uh oh!

wilzbach commented Dec 8, 2016

Uh oh!

wilzbach commented Dec 8, 2016

Uh oh!

CyberShadow commented Dec 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Hackerpilot commented Oct 14, 2016 •

edited

Loading

wilzbach commented Dec 8, 2016 •

edited

Loading

JackStouffer commented Dec 8, 2016 •

edited

Loading