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

Check for most commands if the project is an amethyst project #25

Merged
merged 7 commits into from Mar 29, 2016

Conversation

Projects
None yet
3 participants
@White-Oak
Contributor

White-Oak commented Mar 23, 2016

A function to check if a specified project appears to be a valid amethyst project.

  • Decide if is_amethyst_project should return bool or CmdResult. The latter is more convinient.
  • Current implementation checks only existence of config.yml and yml parsability. Probably it should also check contents, but they are not specified by the current documentation.
  • Tests
    • Fail for build command if not an amethyst project
@White-Oak

This comment has been minimized.

Show comment
Hide comment
@White-Oak

White-Oak Mar 24, 2016

Contributor

pinging @ebkalderon for a review

Contributor

White-Oak commented Mar 24, 2016

pinging @ebkalderon for a review

@ebkalderon

This comment has been minimized.

Show comment
Hide comment
@ebkalderon

ebkalderon Mar 24, 2016

Member

@White-Oak Thanks! Let me finish reviewing #23 first and I'll look at this one.

Member

ebkalderon commented Mar 24, 2016

@White-Oak Thanks! Let me finish reviewing #23 first and I'll look at this one.

@LucioFranco

This comment has been minimized.

Show comment
Hide comment
@LucioFranco

LucioFranco Mar 24, 2016

Member

@White-Oak Should you not check if there is like a project name or something inside the yaml file?

Member

LucioFranco commented Mar 24, 2016

@White-Oak Should you not check if there is like a project name or something inside the yaml file?

@White-Oak

This comment has been minimized.

Show comment
Hide comment
@White-Oak

White-Oak Mar 24, 2016

Contributor

@LucioFranco the template of a new project's config is

---
logging:
    file_path: "new_project.log"
    output_verbosity: medium
    logging_verbosity: debug 

display:
    brightness: 1.0
    fullscreen: false
    resolution: [1024, 768]

We will probably need some docs on that file to see what should be in it.

Contributor

White-Oak commented Mar 24, 2016

@LucioFranco the template of a new project's config is

---
logging:
    file_path: "new_project.log"
    output_verbosity: medium
    logging_verbosity: debug 

display:
    brightness: 1.0
    fullscreen: false
    resolution: [1024, 768]

We will probably need some docs on that file to see what should be in it.

@LucioFranco

This comment has been minimized.

Show comment
Hide comment
@LucioFranco

LucioFranco Mar 24, 2016

Member

@White-Oak Okay maybe we can create an issue to discuss that. Besides that, I think everything here looks good!

Member

LucioFranco commented Mar 24, 2016

@White-Oak Okay maybe we can create an issue to discuss that. Besides that, I think everything here looks good!

@White-Oak

This comment has been minimized.

Show comment
Hide comment
@White-Oak

White-Oak Mar 24, 2016

Contributor

@LucioFranco would you create one?

Contributor

White-Oak commented Mar 24, 2016

@LucioFranco would you create one?

@LucioFranco

This comment has been minimized.

Show comment
Hide comment
@LucioFranco

LucioFranco Mar 24, 2016

Member

@White-Oak yeah i can do that or should I wait for us to move to the new repo?

Member

LucioFranco commented Mar 24, 2016

@White-Oak yeah i can do that or should I wait for us to move to the new repo?

@ebkalderon

This comment has been minimized.

Show comment
Hide comment
@ebkalderon

ebkalderon Mar 24, 2016

Member

@LucioFranco From what I can understand, the rollover is seamless. Still, you can create it now and then I'll migrate, just in case.

Member

ebkalderon commented Mar 24, 2016

@LucioFranco From what I can understand, the rollover is seamless. Still, you can create it now and then I'll migrate, just in case.

@White-Oak

This comment has been minimized.

Show comment
Hide comment
@White-Oak

White-Oak Mar 28, 2016

Contributor

Merged master and resolved conflicts. Also added check for deploy subcommand.

Contributor

White-Oak commented Mar 28, 2016

Merged master and resolved conflicts. Also added check for deploy subcommand.

@ebkalderon

This comment has been minimized.

Show comment
Hide comment
@ebkalderon

ebkalderon Mar 29, 2016

Member

This looks good, guys! Let's merge.

Member

ebkalderon commented Mar 29, 2016

This looks good, guys! Let's merge.

@ebkalderon ebkalderon merged commit 163210e into amethyst:master Mar 29, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment