Skip to content

refactor: Add TypeScript support #1142

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.

Sign up for GitHub

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

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

refactor: Add TypeScript support #1142

wants to merge 7 commits into from
+77 −15

Conversation

fabiovincenzi
Copy link
Contributor

Overview
This PR introduces TypeScript to git-proxy and refactors relevant code to support it.

Changelog

  • Added TypeScript configuration (tsconfig.json) with essential settings for strict type checking, ES6 compatibility, and JSX support.
  • Added typescript and ts-node to manage TypeScript code compilation and execution.
  • Updated the package.json to include TypeScript dependencies.
  • Converted the main entry file from JavaScript (index.js) to TypeScript (index.ts).
  • Modified the start script to use ts-node for running TypeScript files.

Thanks to @jescalada for the work on fixing CI issues on G-Research#31, which helped guide the necessary changes in this PR.

Related issue: #927

Note: Restored from deleted fork

This PR recreates the original PR #929, which was automatically closed due to accidental fork deletion.

** For discussions and reviews:** See the original PR #929

All commits are identical to the original with preserved git history.

@fabiovincenzi fabiovincenzi linked an issue Aug 4, 2025 that may be closed by this pull request
[Setup] Add TypeScript to the Project G-Research/git-proxy#16
Open
3 tasks
@netlify Netlify
Copy link

netlify bot commented Aug 4, 2025 •
edited
Loading

✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name Link
🔨 Latest commit 9944217
🔍 Latest deploy log https://app.netlify.com/projects/endearing-brigadeiros-63f9d0/deploys/68b56d90dab0db00085e8fc6

@codecov Codecov
Copy link

codecov bot commented Aug 5, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.78%. Comparing base (1ae25b3) to head (9944217).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1142   +/-   ##
=======================================
  Coverage   82.78%   82.78%           
=======================================
  Files          66       66           
  Lines        2783     2783           
  Branches      332      332           
=======================================
  Hits         2304     2304           
  Misses        431      431           
  Partials       48       48

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jescalada
jescalada approved these changes Aug 5, 2025
View reviewed changes
Copy link
Contributor

@jescalada jescalada left a comment

Choose a reason for hiding this comment

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

Reapproving this - it's a lot easier to see the changes now that merge conflicts have been resolved 👍🏼

@coopernetes @06kellyjac I suppose this could go into the first release candidate for v2? Or should we deal with the few remaining TS PRs and get it all in a single release? #960 #1063 #1065

06kellyjac
06kellyjac requested changes Aug 5, 2025
View reviewed changes
Copy link
Contributor

@06kellyjac 06kellyjac left a comment

Choose a reason for hiding this comment

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

Yes, moving to providing js and types from dist should be done in v2.
https://github.com/yeoman/generator/blob/main/package.json#L23-L37

Also there's several useful things within git-proxy which we expect people to directly import from files e.g. config, which would break & should be exported properly.

package.json Outdated
@@ -2,6 +2,8 @@
"name": "@finos/git-proxy",
"version": "1.19.2",
"description": "Deploy custom push protections and policies on top of Git.",
"main": "dist/index.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

These files don't live in dist due to ./scripts/build-for-publish.sh

@06kellyjac
Copy link
Contributor

considering the majority of commits are just update merges and there's 2 commits with the same message a rebase & cleanup would be ideal

@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Aug 15, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@jescalada
Copy link
Contributor

Wondering if this is ready to go? 🤔

@fabiovincenzi @06kellyjac

@fabiovincenzi
Copy link
Contributor Author

Hi @06kellyjac, what do you think about the latest changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@06kellyjac 06kellyjac Awaiting requested review from 06kellyjac

@jescalada jescalada Awaiting requested review from jescalada

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
automation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Setup] Add TypeScript to the Project
3 participants
@fabiovincenzi @06kellyjac @jescalada