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

Angular 9 | deep imports #93

Closed
GlauberF opened this issue Apr 15, 2020 · 35 comments
Closed

Angular 9 | deep imports #93

GlauberF opened this issue Apr 15, 2020 · 35 comments
Labels
has PR This issue has at least one PR that resolves it.

Comments

@GlauberF
Copy link

Warning: Entry point 'ngx-avatar' contains deep imports into '/var/www/html/pwa-vimbo-new/node_modules/ts-md5/dist/md5'. This is probably not a problem, but may cause the compilation of entry points to be out of order.

@odahcam
Copy link
Collaborator

odahcam commented Apr 16, 2020

Fixed in #79. Waiting for release.

@odahcam odahcam added the has PR This issue has at least one PR that resolves it. label Apr 16, 2020
@biltongza
Copy link

Any ideas on when the release might be?

@odahcam
Copy link
Collaborator

odahcam commented Apr 22, 2020

No.

@mhosman
Copy link

mhosman commented May 5, 2020

I don't think the repo owner is gonna back. Maybe we could create a new repo and new npm package. Something like ngx-avatars (with an s).

@odahcam
Copy link
Collaborator

odahcam commented May 5, 2020

I would not be do radical, at least for now.

@mhosman
Copy link

mhosman commented May 7, 2020

I understand what you say, but the owner has no activity since last 6 months!

@odahcam
Copy link
Collaborator

odahcam commented May 7, 2020

Yeah, that's something. @HaithemMosbahi maybe you could give us some higher access level in here or even consider moving this repo to a free organization.

@jpike88
Copy link

jpike88 commented May 7, 2020

@HaithemMosbahi are you alive?

Might be worth forking this is the owner is still unreponsive

@PowerKiKi
Copy link
Contributor

I tried to contact @HaithemMosbahi on Twitter. If there is no answer in a few weeks, I'll fork the project for my own need (which may or may not be the same as the community need).

@odahcam
Copy link
Collaborator

odahcam commented May 12, 2020

I also tried email, no response yet. We can still publish the package from this repository, just not to the same name, but I can setup continuos deploy as well. Or just let me know if you gonna need help.

@PowerKiKi
Copy link
Contributor

If I take responsibility for releases, then it will be from my own fork under a different project name. It doesn't feel quite right to keep developing in this repo but publish under another name.

@iamVaibhavPatil
Copy link

@PowerKiKi - Thanks.

@neryortez
Copy link

I think we should consider claiming the npm package
https://docs.npmjs.com/using-npm/disputes.html

@PowerKiKi
Copy link
Contributor

On that page, it reads:

but Yusuf doesn’t have the time to deal with it

So indeed it seems this would be a legit case to dispute the package name. However I won't do it myself, as the package name has little importance to me. But maybe @odahcam would be interested in taking over the whole thing ?

@jpike88
Copy link

jpike88 commented May 26, 2020

@HaithemMosbahi bump. Everyone here is waiting for an answer, they deserve it for chipping into this project.

@odahcam
Copy link
Collaborator

odahcam commented May 26, 2020

I'd love to help, I'll certainty be a maintainer if you want me to. But I find a little rude to @HaithemMosbahi to take the package rights from him without even asking after so much work he put on this. I think we'll be much better suited with a new name or a scoped package publication.

Again, I don't think requesting the package name on NPM is wrong, just not polite. But I'll will go with most of you decide.

As a sidenote: I already maintain and contribute to some OS repos of my own, so doing it alone is out of scope as I could run out of time like Haithem. In that case I would rather prefer to maintain it in collaboration with someone, if that's the case. Would you be interested @PowerKiKi ?

@neryortez
Copy link

Hi there everyone. So... I did not mean to sound rude. Sorry about that.

Let me explain:
Part of the process of claiming the package is contacting the current owner.
By suggesting to claim the package I did not mean to diminish @HaithemMosbahi 's work, on the contrary I believe we should try to contact him first before even opening a new repository and/or package.
I find it awkward and confusing to open a new repository and NPM package when people already rely on Haithem's previous work.

To be clear: I think we must try contacting him and keep the work where it already is.

So if he answers: great! Everyone's happy.
If he is unable to answer (or keep working): people having issues with the library won't have to go to anywhere else.

Also, you can count on me to help on collaboration.

@odahcam
Copy link
Collaborator

odahcam commented May 26, 2020

To be clear: I think we must try contacting him and keep the work where it already is.

So, I got no answers, sorry.

Also, you can count on me to help on collaboration.

Glad to hear read that.

Let me explain: [...]

Oh, that sounds much better. So, I don't know, I'm a little undecided of what is right here. On the other hand everything you said is true, so I'm pending to agree with you. As the package owner I would add back Haithem to maintainer alongside with me and probably @PowerKiKi, but most important of all I could automate the deploy proccess so none of you would ever depend on the NPM maintainer itself to publish a version of the package, just a GitHub Release would do the job. Of course I can't maintain the package alone, so Haithem and @PowerKiKi would be added by me at first opportunity as maintainers so they could help when available. That's what I have to say for now, I do think we can do this "appropriation" without hurting Haithem's work or excluding him.

Tell me what you think is the best option to proceed and I'll go with the majority's idea.

@jpike88
Copy link

jpike88 commented May 27, 2020

@HaithemMosbahi is either incapable or unwilling to maintain this library and has not responded to months of attempts to resolve this. I think we should move on without him

@PowerKiKi
Copy link
Contributor

In case of fork, I would like to:

  • drop broken avatars, unless they are trivial to fix (so at least Google according to Avatar from Google does not get displayed any more due to Picasa API no longer supported. #51, but maybe others too), because I am not willing to spend too much time on something that I don't use, or worse release something that I know is broken
  • consolidate internal structure by making all sources async, so AsyncSource class is deleted and Source interface becomes async and probably becomes an abstract class with supporting methods. This way all sources can be handled the same, instead of having two paths like we do now
  • unconditionally make avatar lazy loaded via loading attribute
  • adopt prettier
  • automate releases based on pushed git tags

If you agree with that, I am wiling to join forces. Otherwise I'll fork under our own org and rename the project and component as eco-avatar (we already maintain https://github.com/Ecodev/fab-speed-dial).

If we join forces, where would that be ? and what name ?

@odahcam
Copy link
Collaborator

odahcam commented May 27, 2020

I'm fine with all your points and I though of some things that would be nice:

  • with prettier we do stay compliant with Angular Style Guide (makes it easier to receive contributions).
  • publish our updates over the past months before doing breaking changes (dropping stuff, api refactoring).
  • make custom source more useful by allowing the user to create multiple custom sources and implement avatar-sources like Google by itself, allowing it (the user) to implement any needed web API keys/configuration/etc without depending on the library's APIs.
  • tag versions under semver pattern.

About the name or org, I have none that proper fits our needs right now. I do maintain @zxing-js and other smaller orgs with friends, but I think they're kind of out of context. Said that I don't mind joining a repository of your choice to contribute.

Just for engagement reasons, I think maybe publishing the package under ecodev scope with the name ngx-avatar (same name as current, but scoped) would help folks to migrate and find the package.

To summarize I'm not really picky of details here, you (plural) know what's best and I'm whiling to help.

@neryortez
Copy link

Same for me as @odahcam
I also think the naming will make it easier to migrate.

@PowerKiKi
Copy link
Contributor

So you both vote to keep the component name identical as <ngx-avatar> ?

I feel it could be a little bit confusing if the component has the same name but comes from a different package (presumably @ecodev/ngx-avatar) ?

I'd rather make a clean break and force all users to make a very easy find-and-replace in their entire project. This would also allow to use both packages at the same time in a project (although I have no idea why you'd want to do that).

@mhosman
Copy link

mhosman commented May 27, 2020

Yeah, I think the best way is to fork this and create a new package name. It's fast and easier, and if @HaithemMosbahi appears one day, his repository will be the same.

@jpike88
Copy link

jpike88 commented May 27, 2020

Ready to switch the moment new package name is published!

@odahcam
Copy link
Collaborator

odahcam commented May 27, 2020

@PowerKiKi the scopes work like a namespace these days, where you specify where you want your something-module to come from, so for some the whole point of using scopes end up to being to repeat names under a different context. Also, I like the idea do keep the name since it would be a fork, thus part of the same project, but just being a variant of it.

About the component name, you can always preffix it with the scope name, like <ecodev-ngx-avatar> (kinda long huh?).

Also maybe it's time to stop calling Angular components ngx, this was a convention for Angular 1 and 2+ compatible projects and we don't even support half of them, hehe.

That's jsut some ideas I hope helps we decide right. We do have people here who doesn't care at all about the name, so feel free to choose the names you feel more comfortable with, personally I find all the mentioned good.

@PowerKiKi
Copy link
Contributor

Ok I'll set up things in in the coming days, probably around Monday, to give @HaithemMosbahi a few more days to magically reappear 🧙‍♂️

@HaithemMosbahi
Copy link
Owner

Hello guys,
I am deeply sorry guys for the inconvenience caused by my absence. I lost access to my Github account and it took me some time to get it back. I am really sorry again.
I will publish new version and I will discuss with @odahcam ( if he agrees of course ) the publishing rights of the package in order to prevent this issue in the future.

@neryortez
Copy link

Great to have you back dude!
Will our revolution come to a halt now? xD

@HaithemMosbahi
Copy link
Owner

Version 4.0.0 is published with Angular 9 support. It would be great if you guys could do some testing on your side! Your feedback is highly appreciated.
The new release includes a new feature: Avatar Cache #96 Thanks to @PowerKiKi for his contribution.
I would like to thank you all guys for your contribution to this project!
Last but not least, a big Thank you to @odahcam for keeping this repository alive! I really appreciate it

@PowerKiKi
Copy link
Contributor

Great timing for a great news ! I won't be forking anything then.

And this (unrelated) issue can be closed.

@odahcam
Copy link
Collaborator

odahcam commented Jun 4, 2020

Hey @HaithemMosbahi it's so good to have you around here once again! I surely agree on having access to publish the package. We can discuss that on #92.

@GlauberF
Copy link
Author

GlauberF commented Jun 4, 2020

Glad this has been resolved.
About the option (Cache) I didn't find anything in the documentation / readme, it would be interesting to update with this new option.

@PowerKiKi
Copy link
Contributor

PowerKiKi commented Jun 4, 2020

The cache is entirely transparent. There is nothing to do. See #96 for details.

PS: though it's more of an "error cache", than "image cache" and might not be exactly what you expect...

@GlauberF
Copy link
Author

GlauberF commented Jun 4, 2020

@PowerKiKi thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has PR This issue has at least one PR that resolves it.
Projects
None yet
Development

No branches or pull requests

9 participants