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

Move the command-line analyzer into the main repository #24731

Closed
bwilkerson opened this issue Oct 26, 2015 · 14 comments

Comments

Projects
None yet
6 participants
@bwilkerson
Copy link
Member

commented Oct 26, 2015

I believe that there are four steps in this process:

  • 1. Copy the code from the old repository to the new repository.
  • 2. Fix the build system to build from the new location rather than the old.
  • 3. Move the issues from the old repository to the new repository.
  • 4. Update dartlang docs to point to new README location.
  • 5. Remove the old repository. Make the old repo read-only. (Follow-up: #25192.)

We should wait to start this until 1.13 has shipped. When a step is finished, please mark it and assign this issue to the person responsible for the next step (or me if you don't know who that is).

@pq

This comment has been minimized.

Copy link
Member

commented Oct 30, 2015

@whesse, I thinking about this one:

[2] Fix the build system to build from the new location rather than the old.

Minimally, I expect we need to remove analyzer_cli from DEPS and update dartanalyzer in bin... Anything not obvious to consider? Will gclient sync complain when it leaves third_party? Anything we can do to make things go smoothest?

Thanks in advance!

@whesse

This comment has been minimized.

Copy link
Member

commented Oct 30, 2015

I don't see any problems with that. When people first update after it is removed, gclient sync will tell them to remove the directory manually. They may well miss that, and leave it there. After that, it may keep putting links to it in the out/[modeArch]/packages directory, until it is removed manually. But that should not be a problem.

pq added a commit that referenced this issue Nov 19, 2015

Move `analyzer_cli` into the SDK.
Build changes to follow.

Tracking bug: #24731

BUG=24731
R=brianwilkerson@google.com

Review URL: https://codereview.chromium.org/1464553003 .

pq added a commit that referenced this issue Nov 19, 2015

`analyzer_cli` move to SDK.
Tracking bug: #24731

Note, dartium build changes are in a separate CL: https://codereview.chromium.org/1453413006/

Some tests had to be disabled for want of mockito in the SDK; tracking their reimplementation is here: #24994

BUG=24731
R=whesse@google.com

Review URL: https://codereview.chromium.org/1459683003 .
@pq

This comment has been minimized.

Copy link
Member

commented Nov 20, 2015

Regarding

  1. Move the issues from the old repository to the new repository.

@nex3 : I know you have a clever script to do bulk issue moves... Is that handy?

@nex3

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

My script is very hacky, and right now those hacks are strongly oriented towards moving issues from the SDK repo to a different repo rather than the reverse. Since there are relatively few issues, it would probably be easier to use the non-bulk mover or just do it manually.

@pq

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

Good deal. Thanks!

@pq pq added the FixIt-15Q4 label Dec 7, 2015

@pq

This comment has been minimized.

Copy link
Member

commented Dec 7, 2015

cc @Sfshaza for the dartlang doc update. (Thanks in advance! :))

@pq

This comment has been minimized.

Copy link
Member

commented Dec 7, 2015

cc @kevmoo as a heads-up that as soon as the docs pointers are up to date, I'll want to nuke the old repo and I may need an assist.

@kevmoo

This comment has been minimized.

Copy link
Member

commented Dec 7, 2015

@pq happy to help

@whesse

This comment has been minimized.

Copy link
Member

commented Dec 7, 2015

We need to be able to build old revisions of the SDK, so we cannot remove a
repo that is referred to by previous versions of the DEPS file. Do not
nuke the repo. You could commit deletions of all files, and a changed
README, but I think just changing the README and prohibiting commits (by a
precommit hook?) is better.

On Mon, Dec 7, 2015 at 6:21 PM, Kevin Moore notifications@github.com
wrote:

@pq https://github.com/pq happy to help


Reply to this email directly or view it on GitHub
#24731 (comment).

William Hesse

@Sfshaza

This comment has been minimized.

Copy link

commented Dec 8, 2015

The redirects have been updated. Try: https://www.dartlang.org/tools/analyzer/

@pq

This comment has been minimized.

Copy link
Member

commented Dec 8, 2015

Yay! Thanks @Sfshaza!

@pq

This comment has been minimized.

Copy link
Member

commented Dec 8, 2015

Sounds good @whesse. @kevmoo : do you know the github-foo to do to prohibit commits?

pq added a commit to googlearchive/analyzer_cli that referenced this issue Dec 8, 2015

@pq

This comment has been minimized.

Copy link
Member

commented Dec 9, 2015

I've gone as far as I know how to go on the read-only bit. Here are the current settings.

screen shot 2015-12-09 at 9 11 33 am

I've opened up a follow-up bug (#25192) for @kevmoo for any loose ends.

@pq

This comment has been minimized.

Copy link
Member

commented Dec 9, 2015

🤘

@pq pq closed this Dec 9, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.