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

Added Text Strikethrough #75

Closed
wants to merge 12 commits into from
Closed

Added Text Strikethrough #75

wants to merge 12 commits into from

Conversation

shaheryar1
Copy link
Contributor

Added Text Strikethrough #63 in ink phase. The pencil texture has not been added yet. I am waiting for it to be available in shared lib.

@proofconstruction
Copy link
Contributor

There are a lot of changes here. It looks like some of the pre-commit changes to the code are spread across several commits somehow.

I have a few questions:

  1. Why did you remove the version requirement from numpy in requirements.txt?
  2. Why did you add and remove bookbinding.md?

I'm happy to merge the new strikethrough.py but I want to avoid these other commits that contain the pre-commit changes we already have in the commit history. Can you delete your version of the repository and fork sparkfish/augraphy again, and then make a new PR with just the strikethrough.py?

@shaheryar1
Copy link
Contributor Author

  1. For numpy pip3 wasn't able to find any version greater than 1.9
  2. I removed bookbinding.md documentation as I thought the failed test are because of that file ( it was the issue of pre-commit )

I guess the other files were not modified by pre-commit earlier and running my pre-commit changed those.

I'll look for some solution for this tonight

@shaheryar1
Copy link
Contributor Author

I guess I should've pushed my changes only which were related to strikethrough.py.

@proofconstruction
Copy link
Contributor

For numpy pip3 wasn't able to find any version greater than 1.9

Where did you see this error?

I removed bookbinding.md documentation as I thought the failed test are because of that file ( it was the issue of pre-commit )

In the future, you can git add the files you want to commit, then pre-commit run --all-files before you commit, and it will fix the files first.

I guess I should've pushed my changes only which were related to strikethrough.py.

Yeah, let's try to keep commits small and only do a few things at a time. Big PRs are hard to look through 😁

@proofconstruction
Copy link
Contributor

If you submit a pull request for just the changes related to strikethrough.py, I'll go ahead and merge it. Closing this one for now though.

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