Skip to content

Removed declarative CAS permission attributes and comments (Part 2) #1035

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Jun 24, 2019
Merged

Removed declarative CAS permission attributes and comments (Part 2) #1035

merged 7 commits into from
Jun 24, 2019

Conversation

ojhad
Copy link
Contributor

@ojhad ojhad commented Jun 20, 2019

Part of issue #241

@ojhad ojhad requested review from rladuca and vatsan-madhavan June 20, 2019 21:41
@ghost ghost requested review from ryalanms and stevenbrix June 20, 2019 21:41
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jun 20, 2019
@vatsan-madhavan
Copy link
Member

lgtm.

did you get a chance to run an IL-diff on the resultant assemblies and the pre-change assemblies?

@stevenbrix
Copy link
Contributor

looks good!

@ojhad
Copy link
Contributor Author

ojhad commented Jun 22, 2019

lgtm.

did you get a chance to run an IL-diff on the resultant assemblies and the pre-change assemblies?

I didn't get a chance to do that yet, but I will run it before merging this PR

@vatsan-madhavan
Copy link
Member

I did a quick IL-diff scan and the changes look ok to me. There is some compiler generated code in DirectWriteForwarder that uses SecurityCriticalAttribute (for e.g., global method _atexit_m). We need to figure out a way to resolve these dependencies.

@vatsan-madhavan vatsan-madhavan added this to the Preview milestone Jun 24, 2019
@ojhad
Copy link
Contributor Author

ojhad commented Jun 24, 2019

I did a quick IL-diff scan and the changes look ok to me. There is some compiler generated code in DirectWriteForwarder that uses SecurityCriticalAttribute (for e.g., global method _atexit_m). We need to figure out a way to resolve these dependencies.

Will take a look and include this in the last PR

@ojhad ojhad merged commit e4a212b into master Jun 24, 2019
@grubioe grubioe mentioned this pull request Jun 28, 2019
Closed
@vatsan-madhavan vatsan-madhavan deleted the user/diojha/casattributeremovalpart2 branch August 23, 2019 20:11
@ghost ghost locked as resolved and limited conversation to collaborators Apr 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stevenbrix stevenbrix stevenbrix approved these changes

@rladuca rladuca rladuca approved these changes

@vatsan-madhavan vatsan-madhavan vatsan-madhavan approved these changes

@ryalanms ryalanms Awaiting requested review from ryalanms

Assignees
No one assigned
Labels
PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Milestone
Preview
Development

Successfully merging this pull request may close these issues.
4 participants
@ojhad @vatsan-madhavan @stevenbrix @rladuca