Skip to content
This repository has been archived by the owner on Mar 1, 2019. It is now read-only.

Migrate changes/fixes to original repository of bigtext.js #2

Open
smohadjer opened this issue Oct 4, 2017 · 4 comments
Open

Migrate changes/fixes to original repository of bigtext.js #2

smohadjer opened this issue Oct 4, 2017 · 4 comments

Comments

@smohadjer
Copy link

Hi Alex,

It seems bigtext.js to have active support on github as they immediately fixed the jquery3 compatibility issue that I reported. I recommend you open issues and/or pull requests in original repository for changes you have made here (considering that these changes are improvements and bug fixes rather than customization of original library), so we can retire this fork and use the original repository instead. This also ensures changes/fixes we make are peer reviewed by their developers and that we don't need to maintain two repositories in sync.

@1010lex
Copy link

1010lex commented Oct 4, 2017

Hi,
for bugfixes I'd agree on that. But we have VI specific changes like consideration of Min, Max Font-Size dependend on the viewport. Plus some sort of bugfix - maybe better call it workaround - which is known since 2014. There exist 2 Tickets for that since then.

zachleat#62
zachleat#65

@timomayer
Copy link

@1010lex thanks for the feedback. maybe you can send a PR to the original repo with this fixes and additions if they are merged we are happy, if not, we ll stay with our fork

@1010lex
Copy link

1010lex commented Oct 4, 2017

@timomayer I think when addressing this it should be adressed in multiple PRs with minimal footprint of the changes. So the original maintainer can get reflect those changes easily.

In addition in the origin repository exist already PRs which seem to address the same bug we had to workaround - those are unanswered and unmerged.

zachleat#70
zachleat#71

As I'm currently short on time to prepare a good PR - plus those open PRs exist already - I wouldn't take action for now.

Note: Despite splitting our changes in multiple PR is reasonable it wouldn't help us if not every one gets merged.

@1010lex
Copy link

1010lex commented Oct 25, 2018

Nevermind - by writing this comment I got unassigned.

@smohadjer @timomayer could your please remove me from this Issue? 

I'm neither working for Virtual Identity nor bitExpert anymore - therefore lost edit/leave permissions to this Issue - but it is still listed in my overview. I wouldn't want to have it sticked there for all eternity :)

Best regards, 
Alex

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants