Skip to content

Allow nightly builds to fail#2131

Merged
weiznich merged 5 commits intodiesel-rs:masterfrom
weiznich:fix/allow_nightly_to_fail_on_azure
Aug 6, 2019
Merged

Allow nightly builds to fail#2131
weiznich merged 5 commits intodiesel-rs:masterfrom
weiznich:fix/allow_nightly_to_fail_on_azure

Conversation

@weiznich
Copy link
Member

If a nightly build fails a warning instead of an error is shown in the
build console

@weiznich weiznich force-pushed the fix/allow_nightly_to_fail_on_azure branch 25 times, most recently from 83bd914 to d1cbfa3 Compare August 1, 2019 14:05
If a nightly build fails a warning instead of an error is shown in the
build console
@weiznich weiznich force-pushed the fix/allow_nightly_to_fail_on_azure branch from d1cbfa3 to 33d0188 Compare August 1, 2019 14:30
@weiznich weiznich force-pushed the fix/allow_nightly_to_fail_on_azure branch from 0e0baf6 to 56a0adf Compare August 1, 2019 20:15
@weiznich
Copy link
Member Author

weiznich commented Aug 1, 2019

Hopefully this makes #2117 obsolete.

@weiznich weiznich requested a review from a team August 1, 2019 20:41
@weiznich
Copy link
Member Author

weiznich commented Aug 1, 2019

Would be great if azure pipelines would report that there were warnings. They are shown in the build status over there, but I'm not sure why they are not reported to github

Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Compiletest in Travis is failed, probably this line should be also changed.

- rust: nightly-2019-02-26

@JohnTitor
Copy link
Member

Ugh, ONCE_INIT is deprecated in nightly-2019-08-01 so compiletest is failed again...

@JohnTitor
Copy link
Member

JohnTitor commented Aug 1, 2019

This PR seems good to me so we can merge this now then we can go to #2108, or this can also include its changes (or add --cap-lints=warn to travis' compiletest).

Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Some targets are still queued on GitHub but all green on Azure Pipelines, lgtm

@JohnTitor
Copy link
Member

Can we go? Or should we wait for another review?

@weiznich
Copy link
Member Author

weiznich commented Aug 5, 2019

I would like to hear at least another opinion about the use of --cap-lints here.

@Eijebong
Copy link
Member

Eijebong commented Aug 5, 2019

So IIUC, the cap-lints part is to make depreciation warning stay as warnings even with a deny(deprecated) ?

@weiznich
Copy link
Member Author

weiznich commented Aug 5, 2019

So IIUC, the cap-lints part is to make depreciation warning stay as warnings even with a deny(deprecated) ?

Yes, but only for the nightly compiler. It is basically meant as a poor mans allow_failures: nightly. Ideally azure would indicate that some job generates warnings, but I've not found any way to do this yet.

@Eijebong
Copy link
Member

Eijebong commented Aug 5, 2019

That would not work on the event of something erroring for real then ? That's kind of a bummer because a dependency might be broken on nightly for some reasons and we don't want that to make the build red in that case either

@weiznich
Copy link
Member Author

weiznich commented Aug 5, 2019

Right, if we want to prevent that we would need to set the this return here to true. I'm not sure if we really want to do that, because ~all of our breakage from the last time was basically rustc introducing some new lint/deprecation/… in nightly.

@Eijebong
Copy link
Member

Eijebong commented Aug 5, 2019

Yeah but I've seen stuff erroring due to errors in dependencies in the past on other projects. I guess this is not common enough to worry about especially since it's fixed quickly most of the time.

I wish rustc would add a cfg for nightly compilers so we could do #[cfg(not(nightly), deny(warnings))] or something...

I guess this is fine then 👍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants