Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Revert EncodingForwarder #10805

Merged
merged 22 commits into from
Apr 8, 2017
Merged

Conversation

tarekgh
Copy link
Member

@tarekgh tarekgh commented Apr 7, 2017

No description provided.

@tarekgh
Copy link
Member Author

tarekgh commented Apr 7, 2017

@jkotas this is the same change we did in 2.0 and in corert but now we need to do it in 1.1. please try to reply quick as today is the deadline to get this in for 1.1. thanks for your help.

cc @danmosemsft

@jkotas
Copy link
Member

jkotas commented Apr 7, 2017

This is churning a lot more than just the encoding forwarder changes (e.g. the delta is removing the [System.Security.SecurityCritical] attributes.). The unnecessary churn looks risky for servicing fix to me.

@tarekgh
Copy link
Member Author

tarekgh commented Apr 7, 2017

@jkotas what exactly the changes you are seeing not related so I can revert it? is it removing the security attributes? or something else?

@tarekgh
Copy link
Member Author

tarekgh commented Apr 7, 2017

ok, I'll return back the security stuff.

@jkotas
Copy link
Member

jkotas commented Apr 7, 2017

Ideally, the change would be clean reverts of the commits that introduced the EncodingForwarder so that it is easy to verify that nothing else fell through the cracks.

@tarekgh
Copy link
Member Author

tarekgh commented Apr 7, 2017

Ideally, the change would be clean reverts of the commits that introduced the EncodingForwarder so that it is easy to verify that nothing else fell through the cracks.

That is what I have done in the first place but I have manually after that removed the security attributes. so I am undoing this now. will send update soon

@jkotas
Copy link
Member

jkotas commented Apr 7, 2017

I still see bits and pieces of 2.0 changes in the delta.

Here is what I have done:

git checkout release/1.1.0
git revert 467f06d6f01deb4905a901a9d337a8dfdef817e1
git revert a64020c8825770ccc8a83d200783a979fbac6434
git revert a4192dc80352345be2c250b4c33b5b72680069d6
git revert 4fd1bfef80fbd4ccb7625196cd841f4fa6415282
git revert 1f9cd6744fcc4c14ee402ba24e26a1b9e4ce8fcc
git revert e1818e622933da083a241963e69f36d5d6d89063
git revert ad1ad35dc03aad4822db57bcbbe1ce7b9f4958ce
git revert f09b521e334fcdf6c279466724a2e6b8f4642cce
git revert 95ff597b07cf40532f6af52beab076bd80332b0f
git revert 77fa0fb69c9113909fbbe3f896779eab26d9e250
git revert 372fcd3cfc8115233040a787e36d020edd919a22
git revert ab47c2281299f464ecddb1d04c292a37b96646dc
git revert 860434db541daf533b43491280774dc4274590b8
git revert f83ecbead207000825555393287dafbca8259137
git revert 6de8ede4e8ca98a382b066ba45a172e3f8ce2821
git revert 7c771b8b8462d0b95130cabfe0de5efbf64278bc
git revert 088943185a0469d936a8c0b6e235bdc715e6fc25
git revert fa55fd74ae3e9374468d32db7371c82e6ae13593
git revert 56c687e0afefc6c223dc0d296a4c64ab5465ebdd
git revert 9668b5f6275f9dbf24ecd0767d5d4817cde16322
git revert e61479088e1ba1f1d8756b8da1a352d518c015c8
git revert ebfc3ce64205a2a5b14c9fe487659ed02c1f3486

And then I have compared it with what you got.

tarekgh added 22 commits April 7, 2017 15:03
@tarekgh tarekgh force-pushed the Build1.1Privates branch from fafedb1 to aa6a17d Compare April 7, 2017 22:09
@tarekgh
Copy link
Member Author

tarekgh commented Apr 7, 2017

@jkotas let me know know if you still see any differences. thanks.

@tarekgh
Copy link
Member Author

tarekgh commented Apr 7, 2017

test Linux ARM Emulator Cross Release Build please

@tarekgh
Copy link
Member Author

tarekgh commented Apr 7, 2017

test OSX x64 Checked Build and Test please

@danmoseley danmoseley merged commit 8123965 into dotnet:release/1.1.0 Apr 8, 2017
danmoseley added a commit that referenced this pull request Apr 8, 2017
@danmoseley
Copy link
Member

@tarekgh I didn't notice this was for 1.1. I'll create a revert PR. Apologies. Let me know whether it was in fact approved, and I'll not submit the revert.

danmoseley added a commit that referenced this pull request Apr 8, 2017
danmoseley added a commit that referenced this pull request Apr 9, 2017
danmoseley added a commit that referenced this pull request Apr 9, 2017
danmoseley added a commit to danmoseley/coreclr that referenced this pull request Apr 9, 2017
@danmoseley
Copy link
Member

Replaced by #10829

tarekgh pushed a commit that referenced this pull request Apr 10, 2017
@karelz karelz modified the milestone: 1.1.x Aug 28, 2017
@tarekgh tarekgh deleted the Build1.1Privates branch March 25, 2019 15:29
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.

5 participants