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

Port relevant changes from ownCloud fork #139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fmoc
Copy link

@fmoc fmoc commented Jul 2, 2021

Closes #135.

I am not really familiar with this project, but I took the time to carefully reviewed all differences and reduced the changeset to a minimum.

Please feel free to review the PR and tell us whether some of the changes can be excluded, I'll happily update the PR.

By the way, you may want to look into black. I can highly recommend this auto formatter.

@fmoc fmoc marked this pull request as draft July 2, 2021 16:06
@fmoc fmoc changed the title WIP: Port relevant changes from ownCloud fork Port relevant changes from ownCloud fork Jul 2, 2021
@michaelstingl
Copy link

@SamuAlfageme who can help here?

@SamuAlfageme
Copy link
Contributor

@fmoc @michaelstingl thanks for taking the time to review the status of your fork! I'll go through it and merge it 🚀 (It's no longer a draft PR, right?)

By the way, you may want to look into black. I can highly recommend this auto formatter.

Thank you for the tip too, did you run the content of the repo against it? Maybe I can introduce it as some CI for this repo.

@fmoc
Copy link
Author

fmoc commented Jul 5, 2021

It's no longer a draft PR

Feel free to remove the draft status. It's a draft because I wanted to make sure you have a look at the proposed changes. As said, I'm not really into this repository, I can't really tell whether all ported changes make sense. I really hope they do, though.

did you run the content of the repo against it?

No, I have not. I would have had to make a huge reformat commit first, but that'd ruin the Git history to some extent. Not everyone's a fan of that. IMO, you need to establish such formatters from the beginning, or only ever apply it to changes when you introduce it later.

@SamuAlfageme SamuAlfageme self-requested a review July 8, 2021 20:47
@SamuAlfageme SamuAlfageme marked this pull request as ready for review July 12, 2021 13:48
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.

Check https://github.com/owncloud/smashbox for upstreamable changes
3 participants