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

chore: max-statements warning in src/cli/init/index.ts #1729

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

tamirazrab
Copy link
Contributor

Description

...

End to End Test:
(See Pepr Excellent Examples)

Related Issue

Fixes #1693

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@tamirazrab tamirazrab requested a review from a team as a code owner January 27, 2025 13:34
Copy link
Contributor

@samayer12 samayer12 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! While reviewing, I noticed that genPeprTS() wasn't actually doing anything that couldn't be solved by referencing a const. So, as a bonus, the refactored setupProjectStructure() function only requires two input parameters.

Copy link

codecov bot commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.08%. Comparing base (7a787b9) to head (e09476a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1729      +/-   ##
==========================================
+ Coverage   82.05%   82.08%   +0.03%     
==========================================
  Files          52       52              
  Lines        2212     2211       -1     
  Branches      470      470              
==========================================
  Hits         1815     1815              
+ Misses        368      367       -1     
  Partials       29       29              
Files with missing lines Coverage Δ
src/cli/init/templates.ts 72.72% <100.00%> (+2.13%) ⬆️

@tamirazrab
Copy link
Contributor Author

Thanks for the PR! While reviewing, I noticed that genPeprTS() wasn't actually doing anything that couldn't be solved by referencing a const. So, as a bonus, the refactored setupProjectStructure() function only requires two input parameters.

Looks much clean now.

@cmwylie19 cmwylie19 changed the title Resolve max-statements warning in src/cli/init/index.ts chore: max-statements warning in src/cli/init/index.ts Jan 28, 2025
@cmwylie19 cmwylie19 enabled auto-merge January 28, 2025 13:23
Copy link
Contributor

@cmwylie19 cmwylie19 left a comment

Choose a reason for hiding this comment

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

Something is not getting generated during the npx pepr build -i pepr:dev as CI is not passing for 2 jobs consistently

This needs a little investigation

https://github.com/defenseunicorns/pepr/actions/runs/13012979379/job/36295073948?pr=1729

One way to test the build is create the pepr test module:

pepr $> npm test

When the pepr-test-module directory is created you can test the build. This allows you to step through the build code and make sure the dist folder is there with the correct files. I will try to find some time to look into this too very soon because I am not sure why the code I am seeing may not be generating some build artifact

pepr/pepr-test-module $> npx ts-node ../src/cli.ts build 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

Resolve max-statements warning in src/cli/init/index.ts
3 participants