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

Update scripts #99

Closed
wants to merge 5 commits into from
Closed

Conversation

bourdakos1
Copy link
Member

@bourdakos1 bourdakos1 commented Apr 7, 2021

This PR:

  • removes the unused makefile
  • adds a watch script to the package.json
  • converts create-release.py to typescript
  • adds a build step to the release script (create-release.py doesn't publish a valid package)
  • updates the help description to match the actual functionality
  • adds a --dry-run flag to test run without doing anything destructive

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

@bourdakos1 bourdakos1 requested a review from lresende April 7, 2021 01:48
@lresende
Copy link
Member

lresende commented Apr 7, 2021

What was wrong with create-release.py?

@bourdakos1
Copy link
Member Author

It was the only piece of code that used python. To run it, you need to have python installed, set up an env, and figure out what dependencies need to be installed (semantic_version needed to be pip installed). Additionally, linting and formatting is set up for typescript, but we don’t have anything similar set up for python (and feels like overkill to set up for one script). I would be a bit more okay to keep create_release.py if it worked out-of-the-box, but still would prefer to keep this project one language.

@bourdakos1 bourdakos1 marked this pull request as draft April 7, 2021 18:19
@bourdakos1
Copy link
Member Author

@lresende I broke up the release script into version, changelog and publish scripts. WIth the intention of running version + changelog and then creating a PR to be reviewed. Once approved/merged we would run the publish command which would tag a release and publish the packages. We could either run publish manually or set up ci to handle publishing

@codecov-io
Copy link

Codecov Report

Merging #99 (7dba632) into master (33d8d13) will decrease coverage by 0.60%.
The diff coverage is 86.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #99      +/-   ##
==========================================
- Coverage   88.53%   87.93%   -0.61%     
==========================================
  Files          22       29       +7     
  Lines         759     1094     +335     
  Branches      172      245      +73     
==========================================
+ Hits          672      962     +290     
- Misses         70       97      +27     
- Partials       17       35      +18     
Impacted Files Coverage Δ
...pipeline-editor/src/ThemeProvider/useSystemInfo.ts 50.00% <50.00%> (ø)
...kages/pipeline-editor/src/PipelineEditor/index.tsx 46.82% <62.50%> (+4.37%) ⬆️
...ges/pipeline-editor/src/SplitPanelLayout/index.tsx 84.74% <70.00%> (-15.26%) ⬇️
...ckages/pipeline-editor/src/ThemeProvider/index.tsx 78.94% <78.94%> (ø)
...e-editor/src/CustomFormControls/BooleanControl.tsx 96.55% <88.88%> (-3.45%) ⬇️
...itor/src/CustomFormControls/StringArrayControl.tsx 97.70% <89.28%> (-2.30%) ⬇️
...ckages/pipeline-editor/src/ThemeProvider/styles.ts 93.75% <93.75%> (ø)
...es/pipeline-editor/src/TabbedPanelLayout/index.tsx 97.61% <95.83%> (-2.39%) ⬇️
...line-editor/src/CustomFormControls/FileControl.tsx 100.00% <100.00%> (ø)
packages/pipeline-editor/src/IconButton/index.tsx 100.00% <100.00%> (ø)
... and 14 more

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 c2eefb3...7dba632. Read the comment docs.

@lresende
Copy link
Member

lresende commented Nov 3, 2021

Outdated

@lresende lresende closed this Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants