DMNO Action MVP #3
Conversation
| const serviceName = inputs.serviceName || 'root' | ||
| args.push(`--service ${serviceName}`) | ||
| // service selection | ||
| if (inputs.serviceName) { |
There was a problem hiding this comment.
Ill fix the defaulting behaviour in dmno directly
| @@ -82,7 +83,7 @@ export async function run(): Promise<void> { | |||
|
|
|||
| // Execute dmno and capture output directly | |||
| const { stderr } = await getExecOutput( | |||
| `${packageManager} exec dmno resolve ${createArgString(inputs).join(' ')}`, | |||
| `${packageManager} exec -- dmno resolve ${createArgString(inputs).join(' ')}`, | |||
There was a problem hiding this comment.
could also try calling ./node_modules/.bin/dmno directly.
We can explore this later. It might be good for a non js repo, and have it just work without needing a package.json file
There was a problem hiding this comment.
the issue is now we rely on it already being a js repo, definitely something to explore for the future. we could detect if it's missing and install it
| uses: ./ | ||
| with: | ||
| milliseconds: 2000 | ||
| # - name: Test Local Action |
There was a problem hiding this comment.
should be able to make this work now - and can use the example config to actually do a basic smoke test
CONTRIBUTING.md
Outdated
| @@ -0,0 +1,161 @@ | |||
| # Create a GitHub Action Using TypeScript | |||
| - name: Print Output | ||
| id: output | ||
| run: echo "${{ steps.test-action.outputs.time }}" | ||
| name: Example Multi-job DMNO GH Action |
There was a problem hiding this comment.
maybe some clarification about loading the current config into env vars, versus needing to do something with multiple steps?
There was a problem hiding this comment.
might be better suited to a guide somewhere? I think the main confusion is multi-step (which basically everyone needs to do) vs multi-job which seems maybe less necessary?
| steps: | ||
| # REST OF WORKFLOW | ||
| - uses: dmno-dev/dmno-gh-action@v1 | ||
| id: dmnoStep |
There was a problem hiding this comment.
is there something weird with these spaces?
There was a problem hiding this comment.
could be a result of the linting?
|
|
||
| expect(runMock).toHaveBeenCalled() | ||
| it('calls run when imported', async () => { | ||
| await import('../src/index.js') |
There was a problem hiding this comment.
🤦 tests like this are so silly... fine to leave it since it was part of the template though
There was a problem hiding this comment.
yeah i didn't write that lol
package.json
Outdated
| "test:watch": "vitest", | ||
| "test:coverage": "vitest run --coverage", | ||
| "all": "npm run format:write && npm run lint && npm run test && npm run package", | ||
| "make-badge": "npx make-coverage-badge --output-path ./badges/coverage.svg" | ||
| }, | ||
| "license": "MIT", | ||
| "jest": { |
package.json
Outdated
| "typescript": "^5.7.2" | ||
| } | ||
| "typescript": "^5.7.2", | ||
| "vitest": "^1.0.0" |
There was a problem hiding this comment.
any reason for the old version of vitest?
There was a problem hiding this comment.
no idea, I think I just did an npm install, can update
| @@ -1,7 +1,7 @@ | |||
| /** | |||
| * The entrypoint for the action. | |||
| */ | |||
| import { run } from './main' | |||
| import { run } from './main.js' | |||
There was a problem hiding this comment.
would be nice to not have to use .js extensions in ts files. IIRC, the fix involves changing the tsconfig?
There was a problem hiding this comment.
yeah I think so, I was balancing not breaking the existing ts stuff elsewhere, but I don't like the .js extension either
README.md
Outdated
| - `emit-env-vars`: Whether to emit environment variables, defaults to true | ||
| - `output-vars`: Whether to also provide the variables in the output, defaults | ||
| to false | ||
| - `skip-regex`: The regex to skip config for, defaults to empty string |
There was a problem hiding this comment.
dont have to do it right now, but we may want to think through these options - and its related options on dmno itself. Like does skipping it mean we dont resolve it at all? or just not export it? I could imagine emit-env-vars taking an array of strings, which could be treated as either strings or regexes, etc...
There was a problem hiding this comment.
skipping currently means we just skip outputting it either in the env vars or the output vars, we can revisit when dmno itself lets us skip in a similar way
There was a problem hiding this comment.
yep! just making that mental note to come back to this when we do :)
No description provided.