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

Privacy Imports #12

Open
mateor opened this issue Apr 4, 2013 · 1 comment
Open

Privacy Imports #12

mateor opened this issue Apr 4, 2013 · 1 comment

Comments

@mateor
Copy link
Member

mateor commented Apr 4, 2013

CollegeDev handles this a little differently, but all privacy imports (for ease of patching) should go right at the top of the file, under package.

So:

/*
 * Copyright (C) 2008 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package android.telephony;

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
BEGIN PRIVACY

I have seen too many patch failures at dumb places like this. This is what broke our patches between 4.1.1 and 4.1.2, they imported a new class and it fudged our patches. If we standardize like the above, that goes away. I have seen it many times, it is not infrequent. And it should not be at the end, because that code changes often as well. At the top, the only time it will break is if they move the location, which will break them anyway.

@mateor
Copy link
Member Author

mateor commented Apr 4, 2013

And while I am at it, I want to do the same thing with the variable declarations. I know that there is a standard grouping, but this seems like a perfect instance of substance over style. If we put the variables directly under the class declaration, upstream adding or subtracting variables doesn't have to break our patches. In lists like this, each variable is a line instead of part of a line, so it is much easier to break patches because each change completely wipes the context.

So instead of this:

    private boolean mAutomaticPerformanceSnapshots = false;
    private PerformanceCollector mPerformanceCollector;
    private Bundle mPerfMetrics = new Bundle();

    //---------------------------------------------------------------------------------------------
    private PrivacySettingsManager mPrvSvc;
    //---------------------------------------------------------------------------------------------

    public Instrumentation() {

All patches are better served by this:

public class Instrumentation {
    //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
    BEGIN PRIVACY

...

Even if it goes against style here. We declare our Privacy sections in a big flashy way now, I think anyone who needs to understand this will be able to.

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

No branches or pull requests

1 participant