Skip to content
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

Configuration is updated to use files or environment variables. #93

Merged
merged 2 commits into from
Aug 28, 2018

Conversation

Skarlso
Copy link
Member

@Skarlso Skarlso commented Aug 26, 2018

No description provided.

@Skarlso Skarlso requested a review from michelvocks August 26, 2018 12:16
@codecov-io
Copy link

codecov-io commented Aug 26, 2018

Codecov Report

Merging #93 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #93   +/-   ##
=======================================
  Coverage   64.54%   64.54%           
=======================================
  Files          22       22           
  Lines        1887     1887           
=======================================
  Hits         1218     1218           
- Misses        537      538    +1     
+ Partials      132      131    -1
Impacted Files Coverage Δ
scheduler/scheduler.go 74.28% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7409114...d861782. Read the comment docs.

README.rst Outdated

For example:

.. code:: bash
Copy link
Member Author

Choose a reason for hiding this comment

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

@michelvocks How do I format code? :O This isn't showing up. :O

Copy link
Member

Choose a reason for hiding this comment

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

You need to add an empty line after .. code:: bash for example:

.. code: bash

#!/bin/bash
echo hello world

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, thanks 🙇 :)

@Skarlso
Copy link
Member Author

Skarlso commented Aug 26, 2018

Deals with #32

@Skarlso Skarlso force-pushed the define_runtimeflags_through_environment_variables branch from 7296b9b to 08287b0 Compare August 26, 2018 18:51
@Skarlso Skarlso force-pushed the define_runtimeflags_through_environment_variables branch from 08287b0 to cc7bcf8 Compare August 26, 2018 19:40
@Skarlso Skarlso added the Ready To Merge PR is ready to be merged into master label Aug 26, 2018
@michelvocks
Copy link
Member

michelvocks commented Aug 27, 2018

What do you think about moving the documentation part out of the README.rst file and into the docs? 😉
This information is important but I'm not sure if it is really necessary to add this to the README.rst file?
Otherwise LGTM 🤗

Edit: link to docs repo: https://github.com/gaia-pipeline/docs/tree/master/content

@Skarlso
Copy link
Member Author

Skarlso commented Aug 27, 2018

@michelvocks Hum. Where do you think I should put it? Should I create a new section?
Should I add it here? https://github.com/gaia-pipeline/docs/blob/master/content/getting-started/install-gaia.md

@michelvocks
Copy link
Member

https://github.com/gaia-pipeline/docs/blob/master/content/getting-started/install-gaia.md is fine I guess. 😄

Copy link
Member

@michelvocks michelvocks left a comment

Choose a reason for hiding this comment

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

LGTM ❤️

@michelvocks michelvocks merged commit 07cf7a3 into master Aug 28, 2018
michelvocks pushed a commit to gaia-pipeline/docs that referenced this pull request Aug 28, 2018
@michelvocks michelvocks deleted the define_runtimeflags_through_environment_variables branch August 29, 2018 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready To Merge PR is ready to be merged into master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants