Skip to content
This repository has been archived by the owner on Nov 5, 2019. It is now read-only.

Removing old_run module. #285

Merged
merged 1 commit into from
Aug 26, 2015
Merged

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Aug 25, 2015

Has been deprecated since 700f13f which occurred on July 2, 2013.

@craigcitro Do you see any reason not to remove this module?

(I debated getting 100% coverage for it in #212 and this seemed like an easier path forward)

Has been deprecated since 700f13f
which occurred on July 2, 2013.
@craigcitro
Copy link
Contributor

I suspect a number of people still use it.

@dhermes
Copy link
Contributor Author

dhermes commented Aug 25, 2015

OK @nathanielmanistaatgoogle shall we

  • Remove it anyway and instruct those people to upgrade to using run_flow (we could even provide a snippet that would show them how to use run_flow with gflags without having to provide the old_run module)
  • Un-deprecate the old_run module
  • Do nothing but make sure it gets 100% test coverage for Get to 100% code coverage #212

@dhermes
Copy link
Contributor Author

dhermes commented Aug 25, 2015

@craigcitro I assume you mean internal to Google yes?

@craigcitro
Copy link
Contributor

i'm sure there are some in google -- but actually, i suspect there are plenty outside. i wager some folks just silenced or ignore the deprecation warning and have been happy their code is working with no work on their part.

i don't see "we need 100% test coverage" as a good reason to potentially break them?

@dhermes
Copy link
Contributor Author

dhermes commented Aug 25, 2015

@craigcitro That isn't the reason for breaking them. It just came up because I was looking at the module and realized it's been deprecated for 2 years 1 month (this library is only 5 years 4 months old: ed13252)

@nathanielmanistaatgoogle
Copy link
Contributor

I approve this commit and am fine with "we no longer wish to be responsible for this code and have given more than enough advanced warning" as a reason for its removal. @craigcitro would that work for you? If I were to offer "a developer who depends on more-than-two-years-deprecated code is already broken and merely doesn't yet know it" as a philosophical proposition, how would you receive it?

@dhermes
Copy link
Contributor Author

dhermes commented Aug 26, 2015

@nathanielmanistaatgoogle I'll let you pull the trigger on the merge if/when this discussion sorts itself out.

@craigcitro
Copy link
Contributor

i don't have strong feelings either way -- i was just answering danny's question about "safe to delete".

i'd be hard-pressed to believe any philosophical propositions about an auth library.

craigcitro added a commit that referenced this pull request Aug 26, 2015
@craigcitro craigcitro merged commit b79d7d5 into googleapis:master Aug 26, 2015
@dhermes dhermes deleted the remove-old-run branch August 26, 2015 03:07
@dhermes dhermes mentioned this pull request Aug 26, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants