Skip to content

Comments

Fix style (space between a .. b) in std/json.d#5560

Merged
dlang-bot merged 1 commit intodlang:stablefrom
wilzbach:fix-circle-stable
Jul 6, 2017
Merged

Fix style (space between a .. b) in std/json.d#5560
dlang-bot merged 1 commit intodlang:stablefrom
wilzbach:fix-circle-stable

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Jul 6, 2017

See also: #5541 (comment)

CC @CyberShadow

(will investigate how it managed to get through Circle)

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@wilzbach wilzbach added the Review:Trivial typos, formatting, comments label Jul 6, 2017
@wilzbach
Copy link
Contributor Author

wilzbach commented Jul 6, 2017

I cherry-picked the commit from #5547 (comment)

@wilzbach
Copy link
Contributor Author

wilzbach commented Jul 6, 2017

(will investigate how it managed to get through Circle)

#5511 was never green on CircleCi:

image

It's simply not required:

image

@CyberShadow
Copy link
Member

Thanks :)

@dlang-bot dlang-bot merged commit 1d07393 into dlang:stable Jul 6, 2017
@wilzbach wilzbach deleted the fix-circle-stable branch July 8, 2017 15:46
@wilzbach
Copy link
Contributor Author

wilzbach commented Jul 8, 2017

Regarding #5547 (comment)

maybe because everyone ignores CircleCI, since it fails quite often

@MartinNowak No AFAICT it simply has never been enforced.
Btw regarding failures - I am not so sure whether this is true anymore. Here are the build statistics for the last month:

image

Phobos builds over the last month in details:

image

There were only four failures in the last thirty days and half of them were our fault:

takes ages to load logs.

That might even be an temporary error on the CircleCi side - I have never experienced this (except when being on my phone).

@CyberShadow @MartinNowak can we require CircleCi to pass for the stable branch as well?

@CyberShadow
Copy link
Member

@CyberShadow @MartinNowak can we require CircleCi to pass for the stable branch as well?

Sounds good to me but it's ultimately up to Martin.

takes ages to load logs.

The UI is clunky and it does take a bunch of scrolling and several clicks to open the relevant section for no apparent reason.

@MartinNowak
Copy link
Member

I'd rather not do that. We want people to be able to merge even with small coverage decreases. Might as well adjust the limits, but I'd rather leave that to common sense.
What needs to be fixed is dlang/dlang-bot#69 instead.

@wilzbach
Copy link
Contributor Author

I'd rather not do that. We want people to be able to merge even with small coverage decreases. Might as well adjust the limits, but I'd rather leave that to common sense.

How is this related? CircleCi is about the linting, public tests etc. - it doesn't fail if the coverage decreases. That's CodeCov to which CircleCi sends the coverage files.

@MartinNowak
Copy link
Member

How is this related? CircleCi is about the linting, public tests etc. - it doesn't fail if the coverage decreases. That's CodeCov to which CircleCi sends the coverage files.

I see, only meant the latter, indeed makes sense for Linting. Any plans to work on dlang/dlang-bot#69 regardless.

@wilzbach
Copy link
Contributor Author

indeed makes sense for Linting.

Enabled then :)

Any plans to work on dlang/dlang-bot#69 regardless.

It's not on my main priority list, I might get to it on the next bot hacking session in August, but no promises.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merge:auto-merge Review:Trivial typos, formatting, comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants