Skip to content

Implemented non-zero exit code when directory initialization fails#142

Open
Mastermindaxe wants to merge 1 commit into
swellaby:masterfrom
Mastermindaxe:issue-fix/#110
Open

Implemented non-zero exit code when directory initialization fails#142
Mastermindaxe wants to merge 1 commit into
swellaby:masterfrom
Mastermindaxe:issue-fix/#110

Conversation

@Mastermindaxe

@Mastermindaxe Mastermindaxe commented Jan 29, 2021

Copy link
Copy Markdown
Contributor

Changes

  • Implemented non-zero exit code when directory initalization fails

Related Issues

Sorry for being absent for long. Life happened :D Thought I'd do some cleaning up. Not sure how to write tests for this behaviour though. Have a good one!

@Mastermindaxe

Copy link
Copy Markdown
Contributor Author

Not sure why the clippy hook fails as it doesn't highlight that error on my machine. The pr.linux job failing doesn't seem to be my doing. Ignore and proceed or should we investigate as it happens in other PRs as well?

@calebcartwright

Copy link
Copy Markdown
Member

Thanks for this! I actually want to think through whether this is still a change we really want to make, though if we do, we'll need to restrict it to running during the lib builds (we wouldn't want to do it for the bin).

Not sure why the clippy hook fails as it doesn't highlight that error on my machine. The pr.linux job failing doesn't seem to be my doing

Don't worry about it for now, not related to your changes. On the CI side we do some caching of certain dev tools that aren't preloaded on the build machines so that the builds can execute more quickly (e.g. don't have to recompile the code coverage tool on every run). However, the cache gets wiped after a while, as has happened here. I need to go in and refresh it.

Ignore and proceed or should we investigate as it happens in other PRs as well?

Don't want to ignore it, but also wouldn't be able to merge this just yet anyway

@Mastermindaxe

Copy link
Copy Markdown
Contributor Author

Thanks for this! I actually want to think through whether this is still a change we really want to make, though if we do, we'll need to restrict it to running during the lib builds (we wouldn't want to do it for the bin).

I see! Do we set an env variable or something to control it? If so I can implement the change and just put it on ready until you decide what to do with it.

@calebcartwright

calebcartwright commented Feb 2, 2021

Copy link
Copy Markdown
Member

I see! Do we set an env variable or something to control it? If so I can implement the change and just put it on ready until you decide what to do with it.

cargo sets various env variables which can be inspected, and it inherently has this knowledge so I'm sure there's a cargo var we can just check programmatically. Actually, now that I think about it, there's no reason to attempt to init during a bin install regardless of what we may or may not do on lib builds. Will open a separate issue for that as it can, and should, be addressed separately (refs #144)

(edit - added issue link)

@codecov

codecov Bot commented Feb 2, 2021

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (e284e15) to head (fdfbf2e).
⚠️ Report is 31 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #142   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          144       142    -2     
=========================================
- Hits           144       142    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@calebcartwright

Copy link
Copy Markdown
Member

Side note, have fixed the cache issue so now the only failing job is clippy. That's a legitimate failure, presumably caused by some updated lints, #145 opened to track resolving that issue

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.

Update build script error behavior

2 participants