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

Moving back from https://github.com/cozy/afero to https://github.com/spf13/afero? #3781

Closed
Peltoche opened this issue Feb 22, 2023 · 12 comments · Fixed by #3783
Closed

Moving back from https://github.com/cozy/afero to https://github.com/spf13/afero? #3781

Peltoche opened this issue Feb 22, 2023 · 12 comments · Fixed by #3783

Comments

@Peltoche
Copy link
Contributor

Peltoche commented Feb 22, 2023

Hi @nono 👋

From what I have understood the repo spf12/afero have been forked in order to fix a Readdir function: spf13/afero@991b95a. This change have been proposed and merged into the main line: spf13/afero#170.

Since then it seems that the main line have received a lot of updated and we didn't maintain the fork. At the moment the fork is 142 commits behind.

Maybe we should considere to move back the the main line and delete this fork?

@nono
Copy link
Member

nono commented Feb 22, 2023

We should look for doing that. I think there were other commits in our fork, but I don't remember what.

@Peltoche
Copy link
Contributor Author

spf13/afero@master...cozy:afero:master to be precise.

Seems to have two main commits:

Return error in Readdir on regular mem file fixing spf13/afero#169 -> This is the one merged (spf13/afero#170)

and

Fix renaming a dir with sub-directories -> There is no linked issues or corresponding pulls requests but the code seems to be present in master too: https://github.com/spf13/afero/blob/12b7d27decdd033919e7e01195e97dd3d9403ce8/memmap.go#L309, the test created in the commit is present in master -> https://github.com/spf13/afero/blob/12b7d27decdd033919e7e01195e97dd3d9403ce8/memmap_test.go#L453

After this check it looks safe to me to move to the main branch. I will create a PR for it and we will see what the CI says

@nono
Copy link
Member

nono commented Feb 23, 2023

Fix renaming a dir with sub-directories -> There is no linked issues or corresponding pulls requests but the code seems to be present in master too: https://github.com/spf13/afero/blob/12b7d27decdd033919e7e01195e97dd3d9403ce8/memmap.go#L309

No, this page says:

This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

And I don't see my code on spf13's master. There is no TestMemFsRenameDir on https://github.com/spf13/afero/blob/master/memmap_test.go, and https://github.com/spf13/afero/blob/master/memmap.go#L298-L323 doesn't look like my patched version.

@nono
Copy link
Member

nono commented Feb 23, 2023

Oh, and I found the PR I have made: spf13/afero#239. And there was also an update here: spf13/afero#320. Let's try again.

@Peltoche
Copy link
Contributor Author

Do you take care of creating the new pull request or should I do it?

@nono
Copy link
Member

nono commented Feb 23, 2023

I have rebased spf13/afero#239 and pinged the maintainer

@nono
Copy link
Member

nono commented Feb 23, 2023

But you can make a PR to spf13/afero to fix their issues with statikcheck. I think that the version of statikcheck they use is no longer compatible with go 1.17 and go1.18.

@nono
Copy link
Member

nono commented Feb 23, 2023

My PR has been merged. Let's hope that the maintainer also make a new release 🤞

@Peltoche
Copy link
Contributor Author

Damn that's quick!

But you can make a PR to spf13/afero to fix their issues with statikcheck. I think that the version of statikcheck they use is no longer compatible with go 1.17 and go1.18.

I don't understand what issue are you talking about?

@Peltoche
Copy link
Contributor Author

The release is already done -> https://github.com/spf13/afero/releases/tag/v1.9.4

@nono
Copy link
Member

nono commented Feb 23, 2023

I don't understand what issue are you talking about?

It was https://github.com/spf13/afero/actions/runs/4250910073/jobs/7392677488, but the maintainer has fixed it with spf13/afero@a6023d2#diff-faff1af3d8ff408964a57b2e475f69a6b7c7b71c9978cccc8f471798caac2c88R19

@Peltoche
Copy link
Contributor Author

I update #3783 to the version v.1.9.4 and we should be ok.

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 a pull request may close this issue.

2 participants