X Tutup
Skip to content

test: Add init manifest script#66

Open
kasbah wants to merge 2 commits intomasterfrom
add-init-manifest-script
Open

test: Add init manifest script#66
kasbah wants to merge 2 commits intomasterfrom
add-init-manifest-script

Conversation

@kasbah
Copy link
Collaborator

@kasbah kasbah commented Jun 19, 2018

No description provided.

@kasbah kasbah force-pushed the add-init-manifest-script branch from b49b2a3 to 10550d0 Compare June 19, 2018 11:34
Copy link
Member

@mcous mcous left a comment

Choose a reason for hiding this comment

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

I've come around to this. Turns out it was 12AM (or whatever) and I wasn't thinking about the benefits of lowering the barrier to entry for adding more integration tests, so I'm sorry about that.

If you don't mind, I've got a few housekeeping requests:

  1. Rename it to init-board-manifest.js
    • We may want to add other scripts to the fixtures
    • .js extension ensures the linter will pick it up
  2. Put init-board-manifest.js in fixtures/bin and wire it up in fixtures/package.json
    • {"bin": {"init-board-manifest": "bin/init-board-manifest.js"}}
    • That way, yarn run init-board-manifest from the top level will be a thing

Then, to address the stuff I brought up in #64, can we more explicitly print out (a prompt would be amazing, but an interactive CLI is a much bigger project) what each setting was for each file? Something like:

please verify the following layer settings are correct

arduino-uno.cmp
> file type:   gerber
> layer type:  top copper

Looks like the current script relies on stdout piping, so that stuff could be fine to send to stderr via console.warn or something

@mcous mcous closed this Mar 9, 2019
@mcous mcous changed the base branch from next to master March 9, 2019 20:33
@mcous
Copy link
Member

mcous commented Mar 9, 2019

Oops, closed automatically when I deleted next. Sorry!

@mcous mcous reopened this Mar 9, 2019
@codecov
Copy link

codecov bot commented Mar 9, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@694eb66). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #66   +/-   ##
=========================================
  Coverage          ?   88.57%           
=========================================
  Files             ?       55           
  Lines             ?     2180           
  Branches          ?        0           
=========================================
  Hits              ?     1931           
  Misses            ?      249           
  Partials          ?        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 694eb66...10550d0. Read the comment docs.

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.

2 participants

X Tutup