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

fix: implement alternative tar.gz on old android (VO-310) #1170

Merged

Conversation

acezard
Copy link
Contributor

@acezard acezard commented Feb 19, 2024

🐛 Bug Fixes

@acezard acezard force-pushed the fix--implement-alternative-handler-for-tar.gz-on-android5/6 branch 3 times, most recently from 9e4fa20 to cd8828f Compare February 19, 2024 14:28
@acezard acezard marked this pull request as ready for review February 19, 2024 14:30
Copy link
Member

@Ldoppea Ldoppea left a comment

Choose a reason for hiding this comment

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

I have too comments about this.

First, why did you create the repository in the cozy-labs org instead of instead of the cozy one? There are already a few react-native forks in the cozy one (cf here)

Also, I would suggest to use only PRs in those repositories, this would help to list all Cozy's edits from a single search (the PR page) in the future.
However this is nit as I don't expect the original repo to suddenly get new commits in the future, so we won't need to do a merge upstream. But keep in mind that for other repos using PRs would ease a lot.

Also I added 2 comments to the new repo's commits, I don't know how to keep track of them as they are not in a PR so I put them here:

@acezard
Copy link
Contributor Author

acezard commented Feb 19, 2024

I have too comments about this.

First, why did you create the repository in the cozy-labs org instead of instead of the cozy one? There are already a few react-native forks in the cozy one (cf here)

Also, I would suggest to use only PRs in those repositories, this would help to list all Cozy's edits from a single search (the PR page) in the future. However this is nit as I don't expect the original repo to suddenly get new commits in the future, so we won't need to do a merge upstream. But keep in mind that for other repos using PRs would ease a lot.

Also I added 2 comments to the new repo's commits, I don't know how to keep track of them as they are not in a PR so I put them here:

you're right it should be in cozy namespace, and use PR rather than commit to main directly. Also keep history of the first change and check why it was made exactly

  • Move to cozy, use PRs

@acezard acezard force-pushed the fix--implement-alternative-handler-for-tar.gz-on-android5/6 branch from cd8828f to bde6c4c Compare February 19, 2024 15:24
@acezard
Copy link
Contributor Author

acezard commented Feb 19, 2024

@acezard acezard requested a review from Ldoppea February 19, 2024 15:25
@acezard acezard force-pushed the fix--implement-alternative-handler-for-tar.gz-on-android5/6 branch from bde6c4c to 62fb567 Compare February 19, 2024 15:26
Copy link
Member

@Ldoppea Ldoppea left a comment

Choose a reason for hiding this comment

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

Perfect, thanks for your efforts 🎉

@acezard acezard merged commit 37a4f3c into master Feb 21, 2024
1 check passed
@acezard acezard deleted the fix--implement-alternative-handler-for-tar.gz-on-android5/6 branch February 21, 2024 07:12
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.

2 participants