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

refactor(templates): Simplify tap template file names with post_gen_project.py hook #2060

Merged
merged 6 commits into from
Nov 21, 2023

Conversation

vicmattos
Copy link
Contributor

@vicmattos vicmattos commented Nov 21, 2023

Copy link

codspeed-hq bot commented Nov 21, 2023

CodSpeed Performance Report

Merging #2060 will not alter performance

Comparing vicmattos:main (a9eb5a2) with main (206c38e)

Summary

✅ 1 untouched benchmarks

Copy link

codspeed-hq bot commented Nov 21, 2023

CodSpeed Performance Report

Merging #2060 will not alter performance

Comparing vicmattos:main (d4f910b) with main (5b84912)

Summary

✅ 1 untouched benchmarks

Copy link

codecov bot commented Nov 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5b84912) 87.45% compared to head (5e2ab6e) 87.45%.
Report is 1 commits behind head on main.

❗ Current head 5e2ab6e differs from pull request most recent head a9eb5a2. Consider uploading reports for the commit a9eb5a2 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2060   +/-   ##
=======================================
  Coverage   87.45%   87.45%           
=======================================
  Files          58       58           
  Lines        4861     4861           
  Branches      763      763           
=======================================
  Hits         4251     4251           
  Misses        429      429           
  Partials      181      181           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@edgarrmondragon
Copy link
Collaborator

Thanks @vicmattos and congrats on your first contribution!

There seems to be a few things causing CI failures:

  • cookiecutter/tap-template/{{cookiecutter.tap_id}}/.github/workflows/test.yml is failing causes it's not valid YAML, which is ok since it's a template but we have to configure the pre-commit hook to ignore it in:
    - id: check-yaml
    exclude: |
    (?x)^(
    cookiecutter/.*/meltano.yml|
    cookiecutter/.*/.pre-commit-config.yaml
    )$
  • The linter is complaining about some of the imports, and some lines are longer than the allowed limit. It might be worth running the template lint checks locally to iterate over and fix the errors, (hint: run nox -rs test_cookiecutter).

@vicmattos
Copy link
Contributor Author

vicmattos commented Nov 21, 2023

Hey @edgarrmondragon! Thanks for your welcoming and feedback.

I've updated the .pre-commit-config.yaml to exclude the template files for CI.

I've also removed the changes I made inside the client files. My main goal was to simplify navigating in the cookiecutter template. I can go back to simplify their contents later.

I've ran the nox automation and seems that everything is correct. Could you double check?

@edgarrmondragon edgarrmondragon changed the title refactor(templates): simplify file names with post_gen_project.py hook refactor(templates): Simplify template file names with post_gen_project.py hook Nov 21, 2023
@edgarrmondragon edgarrmondragon changed the title refactor(templates): Simplify template file names with post_gen_project.py hook refactor(templates): Simplify tap template file names with post_gen_project.py hook Nov 21, 2023
@edgarrmondragon
Copy link
Collaborator

Thanks @vicmattos!

@edgarrmondragon edgarrmondragon merged commit 424e777 into meltano:main Nov 21, 2023
28 of 29 checks passed
edgarrmondragon added a commit that referenced this pull request Nov 21, 2023
@edgarrmondragon
Copy link
Collaborator

I've also removed the changes I made inside the client files. My main goal was to simplify navigating in the cookiecutter template. I can go back to simplify their contents later.

That makes sense, do feel free to send any follow-on PRs 👍

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.

2 participants