Skip to content
This repository was archived by the owner on Feb 2, 2026. It is now read-only.

Conversation

@skoruba
Copy link
Owner

@skoruba skoruba commented Jul 15, 2018

I've added new project with IDS4 that is connected to Asp.Net Core Identity and EF - #30

@xmichaelx - Please take a look. :)

Thank you.

@skoruba skoruba requested a review from xmichaelx July 15, 2018 18:12
@skoruba skoruba requested a review from TomasHubelbauer July 21, 2018 09:18
@skoruba
Copy link
Owner Author

skoruba commented Jul 21, 2018

@TomasHubelbauer - what do you think about it? :)

@TomasHubelbauer
Copy link
Collaborator

Please break the PR into two next time there's this many changes, some of which not reviewable (like wwwroot). I am not sure if wwwroot needs to be tracked (but I think it does, I think the libs come from the template, right? So if that's the case, the non-authored code files could be in a different PR (or in a different commit in the PR so only the actual code changing commits can be reviewed separately - this PR has one big work commit and then a few cleanup/doc update commits and a merge commit). I will review in multiple passes.

@skoruba
Copy link
Owner Author

skoruba commented Jul 21, 2018

Great tip, I agree. :)

@TomasHubelbauer
Copy link
Collaborator

Ah, nevermind, so in this case when I collapsed all the wwwroot stuff, the actual PR was pretty manageable in size so I reviewed in one sweep. Left a few nit comments but in principle it's approved from me. 👍

var grants = await _interaction.GetAllUserConsentsAsync();

var list = new List<GrantViewModel>();
foreach(var grant in grants)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing a space after foreach if you format code that way.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DONE. :)


// this enables automatic token cleanup. this is optional.
options.EnableTokenCleanup = true;
// options.TokenCleanupInterval = 15; // frequency in seconds to cleanup stale grants. 15 is useful during debugging
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be here if commented out? Maybe #if DEBUG?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DONE. :)

@skoruba skoruba removed the request for review from xmichaelx July 24, 2018 10:15
@skoruba skoruba merged commit 492108c into master Jul 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants