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

Remove the term "whitelist" #3457

Closed
wants to merge 1 commit into from
Closed

Conversation

tmeschter
Copy link
Contributor

Bug: Fixes DevDiv Bug 1185799

Customer Scenario
No customer scenario, per se.

There are a number of places where we use the term "whitelist", and one of them (a code comment) was flagged by Policheck. Given that our code is open I've gone ahead and simply fixed them all.

Fix
Several internal types were updated with new names, as well as various fields, locals, and a comment.

Testing
After the fix, the code still passes all of our unit and integration tests.

@ManishJayaswal This is ready to go to ship room.
@srivatsn @mavasani @shyamnamboodiripad @heejaechang @jmarolf @AlekseyTs Could you take a look as well?

@AlekseyTs
Copy link
Contributor

LGTM

@MattGertz
Copy link
Contributor

MoreInfo 6/12 by ML Shiproom. @tmeschter, please talk to @amcasey regarding the references changes.

@MattGertz MattGertz assigned tmeschter and unassigned MattGertz Jun 12, 2015
@amcasey
Copy link
Member

amcasey commented Jun 12, 2015

It looks like it just pretty-printed the references. I'd be tempted to revert the change to make it easier to find the automatically-generated Private elements later.

@tmeschter
Copy link
Contributor Author

Right, VS just reformatted the project file when the file names were updated. I'll undo that part.

We have a couple of places where we check that the set of analyzer
assemblies is internally consistent and complete--that is, for any given
assembly in that set, all of its dependencies are satisfied by other
assemblies in the set.

We expect certain dependencies to be satisfied by the host, and so want
to ignore them if they are not found in the the analyzer assembly set.
We referred to these host-provided dependencies as a white list, but
Policheck does not like that term. Instead, we now refer to these as the
"ignorable assembly list".
@tmeschter
Copy link
Contributor Author

@amcasey @MattGertz @ManishJayaswal I've simply updated the pull request to avoid changes to the <Reference> elements in the project file.

This is ready to go back to ship room.

@tmeschter tmeschter assigned MattGertz and unassigned tmeschter Jun 15, 2015
@Pilchie
Copy link
Member

Pilchie commented Jun 15, 2015

Approved 6/15 by ML Shiproom.

Approved pending a successful Jenkins run.

@tmeschter
Copy link
Contributor Author

Blah. I opened this pull request against master instead of stabilization. I'll close this pull request and open a new one.

@tmeschter
Copy link
Contributor Author

This was handled by #3507.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants