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

GitHub integration #5

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

Conversation

DCCoder90
Copy link

Current work in progress.

Adding Github sync support.

@DCCoder90
Copy link
Author

DCCoder90 commented Aug 9, 2021

Already have the sync functionality built but the setup UI looks a little "clunky", due to the added fields, and I would like to address that prior to submitting the PR for review.

@riedeljan
Copy link
Owner

riedeljan commented Aug 12, 2021

Hi, that's very nice work! Didn't got notified of the PR but will have a look in the evening!

If I can support you somehow just let me know and I'll try to find some time :)

@DCCoder90
Copy link
Author

Sounds great! Sorry, I kept it as a draft PR because I wanted to cleanup the Setup UI. If you want me to submit it as an actual request regardless let me know.

@DCCoder90 DCCoder90 marked this pull request as ready for review August 16, 2021 04:52
@DCCoder90 DCCoder90 changed the title WIP: GitHub integration GitHub integration Aug 16, 2021
@DCCoder90
Copy link
Author

@riedeljan I made a few extra changes, and separated out the logic some more. Please have a look and if any changes need to be made feel free to let me know!

@riedeljan
Copy link
Owner

Thank you so much, the changes look good to me so far! They go straight in the direction I wanted to refactor the code anyway - will review later! (: Really busy rn but love them!

Copy link
Owner

@riedeljan riedeljan left a comment

Choose a reason for hiding this comment

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

Hi, I still didn't manage to proper test the additions but at least had the time to review the code changes. Weeks are way too busy rn. Let me know what you think! :) I'm confident we can finish this by the end of the weekend

src/gitProviders/github.tsx Outdated Show resolved Hide resolved
src/gitProviders/github.tsx Outdated Show resolved Hide resolved
src/gitProviders/github.tsx Outdated Show resolved Hide resolved
src/gitProviders/github.tsx Outdated Show resolved Hide resolved
src/gitProviders/github.tsx Outdated Show resolved Hide resolved
src/gitProviders/github.tsx Outdated Show resolved Hide resolved
src/gitProviders/github.tsx Outdated Show resolved Hide resolved
src/gitProviders/github.tsx Outdated Show resolved Hide resolved
src/gitProviders/gitlab.tsx Outdated Show resolved Hide resolved
src/providerselector.tsx Outdated Show resolved Hide resolved
DCCoder90 added a commit to DCCoder90/insomnia-universal-git that referenced this pull request Aug 22, 2021
@DCCoder90
Copy link
Author

All changes that were requested have since been implemented as of e1fd661

@riedeljan
Copy link
Owner

riedeljan commented Aug 22, 2021

Thank you!

The setup still looks broken which probably comes from the way the nested components are being loaded. I had a look but need to spend some more time with them and think through whether the nesting and code separation could be optimized. Still a good approach so far! 👌🏻

@xadamxk
Copy link

xadamxk commented Dec 14, 2022

Any updates from that review? @riedeljan
EDIT: Just noticed the live version has it natively, nevermind

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

Successfully merging this pull request may close these issues.

3 participants