search

abacritt / angularx-social-login

Social login and authentication module for Angular 17
630 stars 387 forks source link

created v4 support branch for angular 12 based off current master #598

Closed rat-matheson closed 1 year ago

rat-matheson commented 1 year ago

This is a new branch with minor updates in order to work with angular 12. The driving factor for this is I have some software that cannot move off of angular 12. However, the google api's used by angularx-social-login v4 are being deprecated so I need to update them. This has been mentioned in issue 517

I've tested manually best I can but would recommend someone else who knows what to expect from the current master branch to do a test run as well. The current master branch still fails on my machine for some use cases as mentioned in issue 592

rat-matheson commented 1 year ago

Probably obvious but note that this needs to be merged into a new v4-support branch, not master.

Heatmanofurioso commented 1 year ago

Hi @rat-matheson Thanks for your work on this PR.

I don't have permissions to push to NPM on angularx-social-login, so we should maybe consider some tag convention for @abacritt/angularx-social-login regarding V12 support

rat-matheson commented 1 year ago

Thanks for all your work maintaining the library. With regards to the tag convention, do you need me to make any specific changes to the PR/commit? I did update the package name to match the old package so that I wouldn't have any breaking changes.

Heatmanofurioso commented 1 year ago

@rat-matheson The problem is that I don't have permissions to push to the old package, according to the NPM registry.

So, we should probably push to the new package, but with some version that that makes it clear that it isn't an update on the latest version

rat-matheson commented 1 year ago

According to docs.npmjs.com, package.json version must be parseable by node-semver.

What are your thoughts on manipulating the pre-release postfix to achieve the distinction? It's a bit of a hack but we could do something like "1.2.4-ngx12.0" which would mean it was based off of version 1.2.4 but for ngx12. It's pretty far from perfect but perhaps we can do this once and then just leave it and hope everyone can move off of ngx12 and upgrade to the new version in the future.

rat-matheson commented 1 year ago

@Heatmanofurioso - what are your thoughts on my previous comment? Do you think I can follow that suggestion and unblock this issue?

Heatmanofurioso commented 1 year ago

@rat-matheson Sorry for the long time to respond. I'm fine with that approach. 1.2.4-ngx12.0 seems like an appropriate release for this migration then

rat-matheson commented 1 year ago

@Heatmanofurioso - great, thanks. I've made the update. Perhaps you can create a better name for the branch when you merge it into the main project. 'ngx12-support' might be appropriate. Let me know if you need anything else from my end.

rat-matheson commented 1 year ago

@Heatmanofurioso - Hi, sorry to bother you again but is there anything else you need from me to close this issue? I think you will need to create a new branch in the main repository, and then update the pull request destination branch using the method described here. The pull request is based on a bit of an order commit so you need to branch at commit 8cfd9af23508f96c1df62657e33df0cad2fb06f7

aneshodza commented 1 year ago

Is this still alive?

rat-matheson commented 1 year ago

It needs to be merged into the main project. It's blocked on that. However, in the meantime, you could just pull the commit from github rather than npm and use it.

Heatmanofurioso commented 1 year ago

Hi all.

First off, sorry for going AWOL, but I've had some personal issues + swamped with work, but I plan to help on the project more actively again.

@rat-matheson Can you fix these conflicts, please?

rat-matheson commented 1 year ago

No worries about going AWOL. We understand this is not your job.

I'm not sure how to resolve this particular conflict. The issue is with the version number. It needs to go into a new branch where I can replace the old version number without breaking the existing build. However, my fork is based on the master, and it looks like the merge request is going back on there. I don't have permission to create a new branch in the main project and switch the merge request to the new branch

To resolve this, I think you my have to create a new branch in the main project named something like ngx12-support and then merge in my commit using this method: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request

Update: Just tagging @Heatmanofurioso

LiJell commented 1 year ago

I am waiting for this update. thank you for your effort :)

Heatmanofurioso commented 1 year ago

@rat-matheson Sorry for the long wait. I've picked your commits and merged them in a separate PR, since I don't have permissions to solve conflicts on your branch. I'm now deploying the v12 support version