www
/
Eventer
18
0
Fork 0
Code Issues Pull Requests 1 Packages Projects Releases Wiki Activity

gitClient #11

Merged
a34sun merged 13 commits from gitClient into master 2023-04-05 17:33:11 -04:00
Conversation 0 Commits 13 Files Changed 5 +259 -35
tcsrikan commented 2023-03-18 17:41:16 -04:00
Member
Copy Link

Created git client with clone, commit, push, and delete capabilities

Created git client with clone, commit, push, and delete capabilities
tcsrikan added 3 commits 2023-03-18 17:41:17 -04:00
tcsrikan added 1 commit 2023-03-18 17:42:20 -04:00
tcsrikan added a new dependency 2023-03-18 17:44:38 -04:00
#2 Create Git client
a3thakra reviewed 2023-03-20 02:48:56 -04:00
src/useGitClient.ts Outdated
@ -0,0 +3,4 @@
import {cloneRepo, commitRepo, pushRepo, removeRepo} from './gitClient'
//Sample usage
cloneRepo('https://git.csclub.uwaterloo.ca/www/library.git', (temporaryDir) => {
a3thakra commented 2023-03-20 02:48:56 -04:00
Owner
Copy Link

try to build an API that returns promises. that will give you a much better design. reach out on discord if you need ideas! :)

try to build an API that returns promises. that will give you a much better design. reach out on discord if you need ideas! :)
👍 2
snedadah added 1 commit 2023-03-21 00:33:08 -04:00
snedadah reviewed 2023-03-21 00:41:34 -04:00
snedadah left a comment
Owner
Copy Link

Nice work on this!

As Adi mentioned, using promises is much prefered, since it can help us avoid http://callbackhell.com/.

I was typing out some comments to guide you, but I realized that it was a bit tricky, and one thing lead to the next and I ended up just converting the cloneRepo to promises (sorry, didnt mean to take away your fun!). But I still left the other functions for you to conver to promises, hopfully my code helps.

Also, instead of the useGitclientFile, usually for things like this we use tests, so I added a test file, and wrote some tests for the clone. You can run then and inspect the output by running npm test (you might have to do npm install at first)

Please add some simple tests for the other functions as well as your builing to help you develop. You don't have to do extensive tests for the tricky things (overall I decided for this projects a lot of tests are probalby not worth it), but mainly to help you develop. You can write the test and manulayy verify that the correct git action happend (commited, added, etc.) instead of having to do the "useGitClient". (Of course, if you are feeling ambitous, feel free to try doing more complicated automated tests, but its not required)

Nice work on this! As Adi mentioned, using promises is much prefered, since it can help us avoid http://callbackhell.com/. I was typing out some comments to guide you, but I realized that it was a bit tricky, and one thing lead to the next and I ended up just converting the `cloneRepo` to promises (sorry, didnt mean to take away your fun!). But I still left the other functions for you to conver to promises, hopfully my code helps. Also, instead of the `useGitclientFile`, usually for things like this we use tests, so I added a test file, and wrote some tests for the clone. You can run then and inspect the output by running `npm test` (you might have to do `npm install` at first) Please add some simple tests for the other functions as well as your builing to help you develop. You don't have to do extensive tests for the tricky things (overall I decided for this projects a lot of tests are probalby not worth it), but mainly to help you develop. You can write the test and manulayy verify that the correct git action happend (commited, added, etc.) instead of having to do the "useGitClient". (Of course, if you are feeling ambitous, feel free to try doing more complicated automated tests, but its not required)
.vscode/settings.json
@ -0,0 +1,3 @@
{
"editor.formatOnSave": true
snedadah commented 2023-03-21 00:34:46 -04:00
Owner
Copy Link

I enabled this so hopefully so it formats on save for everyone.

I enabled this so hopefully so it formats on save for everyone.
snedadah reviewed 2023-03-21 00:42:40 -04:00
src/gitClient.ts Outdated
@ -0,0 +54,4 @@
});
}
function createAndPublishNewBranch(repoPath) {
snedadah commented 2023-03-21 00:42:40 -04:00
Owner
Copy Link

If I remember correctly, I think yoy were callning this after clone? I don't think this is need to be called inside the clone method, it can be called afterwords seperatly...

If I remember correctly, I think yoy were callning this after clone? I don't think this is need to be called inside the clone method, it can be called afterwords seperatly...
tcsrikan added 4 commits 2023-03-30 11:59:36 -04:00
tcsrikan requested review from snedadah 2023-03-30 12:00:18 -04:00
tcsrikan changed title from gitClient to WIP: gitClient 2023-03-30 12:01:07 -04:00
snedadah reviewed 2023-04-02 14:09:50 -04:00
src/gitClient.ts
@ -0,0 +88,4 @@
try{
execSync(`rm -r ${repoPath}`);
} catch (error){
`Failed to push to repo: ${error.status}
snedadah commented 2023-04-02 14:09:50 -04:00
Owner
Copy Link

I think you forgot a console.error here

I think you forgot a console.error here
tcsrikan added 1 commit 2023-04-04 00:54:26 -04:00
snedadah changed title from WIP: gitClient to gitClient 2023-04-04 20:21:35 -04:00
snedadah reviewed 2023-04-04 20:24:48 -04:00
src/test/gitclient.test.ts
@ -0,0 +18,4 @@
execSync(`cd ${folderPath} && touch newFile.ts`);
await commitRepo(folderPath);
const directoryContents = readdirSync(folderPath);
expect(directoryContents).toContain("newFile.ts")
snedadah commented 2023-04-04 20:24:48 -04:00
Owner
Copy Link

nit: (not importnat) but just wanted to point out this test doesn't actully test that it was commited. Maybe if it ran a git add --all && git reset --hard after the commit, it would test that the commit actually worked.

nit: (not importnat) but just wanted to point out this test doesn't actully test that it was commited. Maybe if it ran a `git add --all && git reset --hard` after the commit, it would test that the commit actually worked.
snedadah marked this conversation as resolved
snedadah added 2 commits 2023-04-05 12:48:51 -04:00
snedadah added 1 commit 2023-04-05 12:53:07 -04:00
snedadah approved these changes 2023-04-05 12:53:57 -04:00
snedadah left a comment
Owner
Copy Link

Nice, great work with this!

I just adjusted that push test to expect the "unauthorized" error so it doesn't fail
when other people run it. I think the way it is right now it still kinda does test that it got to pushing, without actually having to setup correct premissions.

Nice, great work with this! I just adjusted that push test to expect the "unauthorized" error so it doesn't fail when other people run it. I think the way it is right now it still kinda does test that it got to pushing, without actually having to setup correct premissions.
a34sun merged commit 42a77f0a95 into master 2023-04-05 17:33:11 -04:00
a34sun referenced this issue from a commit 2023-04-05 17:33:11 -04:00
gitClient (#11)
a34sun requested review from snedadah 2023-04-05 17:40:06 -04:00
Sign in to join this conversation.
Reviewers
No reviewers
snedadah
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Blocks
#2 Create Git client
www/Eventer
Reference: www/Eventer#11
No description provided.
Delete Branch "gitClient"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?