New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Before running mage task, check whether dependencies are installed #593

Merged
merged 7 commits into from Oct 24, 2018

Conversation

Projects
None yet
2 participants
@nclandrei
Copy link
Contributor

nclandrei commented Oct 21, 2018

Issue: #590

Before running any Mage task, there will be an OS agnostic check to see whether all dependencies are correctly installed.

nclandrei added some commits Oct 21, 2018

@goenning

This comment has been minimized.

Copy link
Member

goenning commented Oct 22, 2018

Hey, @goenning! I submitted the pull request and, funnily enough, one of the dependencies (i.e. air) is missing from the CircleCI build environment, so building the PR fails 😁 I propose that air is also installed on CircleCI as this would be the desired environment (i.e. all the required dependencies should be installed on the build env as well). What do you think?

Nice! So that means it's working as expected! 😁

I have installed air on that Build image, you'll need to change the .circleci/config.yml to use getfider/circleci:0.0.15 instead of 0.0.14, can you do that? That version should have air installed.

I'd also suggest that we put a link to the CONTRIBUTING.md whenever the missing dependencies fail. This might help them on how to install the missing dependencies. Maybe on a new line after that message:

To learn how, visit our contributors guide: https://github.com/getfider/fider/blob/master/CONTRIBUTING.md

One last thing, I'd move the missingDependencies function to the bottom of that file, together with other "Utils" functions that we might have there.

@nclandrei

This comment has been minimized.

Copy link
Contributor

nclandrei commented Oct 23, 2018

Sounds great! On it! 🙂

nclandrei and others added some commits Oct 23, 2018

Merge branch 'missing_tools_notification' of ssh://github.com/nclandr…
…ei/fider into missing_tools_notification

@goenning goenning merged commit cab30ea into getfider:master Oct 24, 2018

5 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: push Your tests passed on CircleCI!
Details
ci/circleci: setup Your tests passed on CircleCI!
Details
ci/circleci: test-server Your tests passed on CircleCI!
Details
ci/circleci: test-ui Your tests passed on CircleCI!
Details
@goenning

This comment has been minimized.

Copy link
Member

goenning commented Oct 24, 2018

Merged! Thanks @nclandrei.

Let's see if we get any feedback to keep improving it! 😄

oxynux added a commit to oxynux/fider that referenced this pull request Oct 24, 2018

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