Skip to content

Conversation

@cevatkerim
Copy link

@cevatkerim cevatkerim commented Dec 29, 2024

  • Add optional PAT field with toggle checkbox in UI
  • Support custom Git server API endpoints (api/v1/repos)
  • Add comprehensive test coverage for PAT authentication
  • Update repository existence check to handle both GitHub and custom Git servers

image

- Add optional PAT field with toggle checkbox in UI\n- Support custom Git server API endpoints (api/v1/repos)\n- Add comprehensive test coverage for PAT authentication\n- Update repository existence check to handle both GitHub and custom Git servers
Copy link
Member

@cyclotruc cyclotruc left a comment

Choose a reason for hiding this comment

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

Very clean minimal implementaiton, I like that the tokens never leave RAM as it reduces the security risk of handling user tokens
Thank you very much

Copy link
Member

@cyclotruc cyclotruc left a comment

Choose a reason for hiding this comment

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

My bad: I missed those 4 lines while merging and I can't seem to be able to push those changes on your PR

@cyclotruc
Copy link
Member

@cevatkerim I was quite reluctant at first to introduce PAT in gitingest but this first implementation seems very reasonable
Thank you very much

I would just like to improve the UX a bit before merging, if you can give me access to your branch I will glaadly implement those changes myself:
image

  • The ingest button should remain as it was before on the same line as the search bar in desktop mode
  • Make the checkbox and new textinput look more like the include/exclude input (mostly smaller)

@cyclotruc
Copy link
Member

Update:

Since gitingest is focused on butter smooth experience rather than having many features, I'm considering not pushing this first step (PAT) in production but instead wait until we have proper OAuth going on with Github login

I'm going to keep this branch as a reference implementation for PAT which I had not much experience with, so your work is still very helpful!

@cyclotruc cyclotruc closed this Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

work in progress This PR is not ready yet but is being worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants