Merge ~bryce/git-ubuntu:project-c97-remove-importppa-importlocal-review into git-ubuntu:master

Proposed by Bryce Harrington
Status: Merged
Merge reported by: Bryce Harrington
Merged at revision: 492f1cc14e5143eaeed8b2aba92e93dbb3030d99
Proposed branch: ~bryce/git-ubuntu:project-c97-remove-importppa-importlocal-review
Merge into: git-ubuntu:master
Diff against target: 1038 lines (+16/-118)
5 files modified
dev/null (+0/-56)
doc/README.repository (+4/-11)
doc/gitubuntu-completion.sh (+0/-29)
gitubuntu/__main__.py (+12/-6)
man/man1/git-ubuntu.1 (+0/-16)
Reviewer Review Type Date Requested Status
Robie Basak Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+383379@code.staging.launchpad.net

Description of the change

Drops importlocal, importppa, and review.

(Dropping of build and lint will be handled separately; they each have some functionality used elsewhere in the codebase so will need some refactoring.)

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:4b8a1e89e663c78b90c50f6e7dc8002e01343039
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/508/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/508//rebuild

review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:fb3cd1c8aa869159ae616c6520cd4af45f6c7f4e
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/509/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/509//rebuild

review: Approve (continuous-integration)
Revision history for this message
Robie Basak (racb) wrote :

This looks good.

My only comment is on the language used on the deprecation notices, is this really a deprecation? To me that means that we're getting rid of these permanently, and that the suggestions are intended to be their long term replacements. I think that:

 * lint and review may be reintroduced one day, but fixed

 * the functionality of import-local and import-ppa may be reintroduced, but perhaps as an overhaul of a more generic "import" command

I'm not sure how much of this story we really need to explain in the error message, but I'd like to not say anything that contradicts the above, and "is deprecated" feels like it does. How about "is not currently available"?

FWIW, the intention of "lint" is that it would be for contributors to use directly and it would be able to do things like suggest automatic fixes to what it finds based on heuristics; "review" would then do the same thing, but more intended for a reviewer and would also rely on "lint" results. So there would be quite a bit of overlap.

Revision history for this message
Robie Basak (racb) wrote :

Feel free to land without another round trip.

review: Approve
Revision history for this message
Bryce Harrington (bryce) wrote :

Thanks for the suggestion, I wondered the same about deprecated.

I've made those corrections and push.

For the build command MP, I'll need to rebase it on these changes and re-verify the tests, and will post it tomorrow.

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches