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

sample: fix samples to works well #175

Merged
merged 1 commit into from
Jan 19, 2023
Merged

sample: fix samples to works well #175

merged 1 commit into from
Jan 19, 2023

Conversation

mkusaka
Copy link
Contributor

@mkusaka mkusaka commented Dec 25, 2022

Hi! Thanks for very interesting project.

I found sample under this repository does't works well. So, I just fix those.

npm install doesn't works

problem

npm install under sample/01-dynamic-workflow-javascript/.circleci/dynamic directory occurs error as follows.

npm ERR! code E404
npm ERR! 404 Not Found - GET https://registry.npmjs.org/@circleci%2fcircleci-config-sdk-ts - Not found
npm ERR! 404
npm ERR! 404  '@circleci/circleci-config-sdk-ts@*' is not in this registry.
npm ERR! 404
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, http url, or git url.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/UserName/.npm/_logs/2022-12-25T17_01_53_293Z-debug-0.log

fix

  • I removed not found package (maybe its typo).
  • I added package from root directory of this repository.

point of discussion

should we add @circleci/circleci-config-sdk from npm registry?

  • After fix, user must build package of this repository before run example.
  • Add @circleci/circleci-config-sdk option doesn't require this operation, but we should bump up to latest for each release.

invalid parameter for docker executor in generated config.

problem

This example's scripts generate following invalid config. docker executor image & resource_class are invalid.

# ...
    docker:
      - image: docker-node # image `docker-node` doesn't exists in any repository.
    resource_class: cimg/node:lts # invalid resource class.
    steps:
      - checkout
# ...

fix

I fixed parameters for CircleCI.executors.DockerExecutor in scripts.

invalid name of generated config

problem

This example is intend to generate dynamicConfig.yml file under .circleci directory. But this example generate .circleci.yml file now.

fix

I renamed generate file name to dynamicConfig.

@ghost
Copy link

ghost commented Dec 25, 2022

👇 Click on the image for a new way to code review
  • Make big changes easier — review code in small groups of related files

  • Know where to start — see the whole change at a glance

  • Take a code tour — explore the change with an interactive tour

  • Make comments and review — all fully sync’ed with github

    Try it now!

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map Legend

Copy link
Contributor

@KyleTryon KyleTryon left a comment

Choose a reason for hiding this comment

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

Appreciate these fixes! There have been a few changes that not all references to have been caught.

@KyleTryon KyleTryon merged commit 00ddfb1 into CircleCI-Public:main Jan 19, 2023
@mkusaka mkusaka deleted the patch branch January 23, 2023 16:58
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