Skip to content

Support of .git folder and .gitignore #85

Closed
herissondev wants to merge 10 commits intoarchethic-foundation:mainfrom
herissondev:handle_git_folder
Closed

Support of .git folder and .gitignore #85
herissondev wants to merge 10 commits intoarchethic-foundation:mainfrom
herissondev:handle_git_folder

Conversation

@herissondev
Copy link
Contributor

As described in #82 aeweb-cli wasn't handeling git folders.
Therefore I added a --git-folder argument.

It works by parsing the .gitignore file and then only uploading files that do not match the gitignore patterns.

to test it simply deploy a git folder as usual but add the --git-folder argument.

@herissondev
Copy link
Contributor Author

path.relative() expects a folder rather than a file as its first argument.
Screenshot_20221128_224212

Hi @apoorv-2204,
I do not get this error with or without the --git-folder command.

My folder also includes a .git/HEAD file and it works fine.
Maybe it is an os based error?
Do you have more info on where the error really occurs?
Thanks

@apoorv-2204
Copy link
Contributor

apoorv-2204 commented Nov 29, 2022

path.relative() expects a folder rather than a file as its first argument.
Screenshot_20221128_224212

Hi @apoorv-2204, I do not get this error with or without the --git-folder command.

My folder also includes a .git/HEAD file and it works fine. Maybe it is an os based error? Do you have more info on where the error really occurs? Thanks

Can you please share your node js version ?

@aime-risson
Screenshot_20221202_121041

Screenshot_20221202_121127

@herissondev
Copy link
Contributor Author

path.relative() expects a folder rather than a file as its first argument.
Screenshot_20221128_224212

Hi @apoorv-2204, I do not get this error with or without the --git-folder command.
My folder also includes a .git/HEAD file and it works fine. Maybe it is an os based error? Do you have more info on where the error really occurs? Thanks

Can you please share your node js version ?

I'm using node js v18.11.0

@apoorv-2204 apoorv-2204 added bug Something isn't working external contribution Contribution by non core team labels Dec 1, 2022
@herissondev
Copy link
Contributor Author

Added the function and it works well !

I just changed the line if (!gitignore.ignores(getAbsolutePath(entry))) to if (!gitignore.ignores(absolutePath)) as we had just calculated it already on the line above.
@apoorv-2204

commands/cli.js Outdated

// if the folder is git folder then we should ignore
// .git and files / folders in gitignore
if (isGitFolder){
Copy link
Member

Choose a reason for hiding this comment

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

We might do the reverse operation.
Instead of expecting this flag, we might look at the existence of the .gitignore file or .git folder to not deploy by default the folder attached.
But if we want to deploy the files referenced in the .gitignore, add a flag to explicit it (--include-git-ignored-files)
In all cases, there is no point to deploy the .git folder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We might do the reverse operation.
Instead of expecting this flag, we might look at the existence of the .gitignore file or .git folder to not deploy by default the folder attached.
But if we want to deploy the files referenced in the .gitignore, add a flag to explicit it (--include-git-ignored-files)
In all cases, there is no point to deploy the .git folder

Hadn't seen your comment sorry, should be fixed now. .git is always ignored, .gitignore files are ignored by default unless --include-git-ignored-files flag is added to the deploy command.

Copy link
Contributor Author

@herissondev herissondev left a comment

Choose a reason for hiding this comment

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

Fix conflicts

@apoorv-2204
Copy link
Contributor

Fix conflicts

#93

Hi @aime-risson , I was unable to push changes on your branch, I created a new pr, with merge of your branch into the new branch. With that your external contribution is still recorded!!.Will also mention as author on last commit,

Cheers 😄

@herissondev
Copy link
Contributor Author

Fix conflicts

#93

Hi @aime-risson , I was unable to push changes on your branch, I created a new pr, with merge of your branch into the new branch. With that your external contribution is still recorded!!.Will also mention as author on last commit,

Cheers 😄

Great thanks!

@Neylix
Copy link
Member

Neylix commented Jan 20, 2023

Related PR #93 has been merged in main branch. Closing this one

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

Labels

bug Something isn't working external contribution Contribution by non core team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants