Skip to content

Make sure "Run As" > "App Engine" shows only on App Engine projects.#1560

Merged
chanseokoh merged 7 commits intomasterfrom
i1556-App-Engine-context-menu-bug
Mar 9, 2017
Merged

Make sure "Run As" > "App Engine" shows only on App Engine projects.#1560
chanseokoh merged 7 commits intomasterfrom
i1556-App-Engine-context-menu-bug

Conversation

@chanseokoh
Copy link
Copy Markdown
Contributor

Fixes #1556, per #1556 (comment).

@codecov-io
Copy link
Copy Markdown

codecov-io commented Mar 9, 2017

Codecov Report

Merging #1560 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master   #1560      +/-   ##
===========================================
+ Coverage     70.89%   70.9%   +0.01%     
  Complexity     1321    1321              
===========================================
  Files           233     233              
  Lines          9002    9002              
  Branches        767     767              
===========================================
+ Hits           6382    6383       +1     
+ Misses         2300    2299       -1     
  Partials        320     320
Impacted Files Coverage Δ Complexity Δ
.../tools/eclipse/test/util/project/ProjectUtils.java 79.59% <0%> (-1.03%) 18% <0%> (-1%)
...clipse/appengine/facets/NonSystemJobSuspender.java 89.18% <0%> (+5.4%) 10% <0%> (+1%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be6a763...2756e1e. Read the comment docs.

@chanseokoh chanseokoh requested a review from briandealwis March 9, 2017 15:56
@chanseokoh
Copy link
Copy Markdown
Contributor Author

Asking @briandealwis to review, since he initially set up the enablement conditions.

Copy link
Copy Markdown
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I greatly appreciate the tests!

</with>
<with variable="activeEditorInput">
<adapt type="org.eclipse.core.resources.IFile">
<with variable="selection">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can drop this outer <with variable="selection"> and just use the <iterate>…</iterate> on value that's passed in (that is likely — but not necessarily — the selection).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, it does work without <with>.

Copy link
Copy Markdown
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

}

// We need regex matching, since the actual menu name is "<number> App Engine".
private boolean appEngineMenuExists(IProject project) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static

@chanseokoh chanseokoh merged commit a32457d into master Mar 9, 2017
@chanseokoh chanseokoh deleted the i1556-App-Engine-context-menu-bug branch March 9, 2017 17:59
@chanseokoh chanseokoh restored the i1556-App-Engine-context-menu-bug branch March 9, 2017 22:18
@chanseokoh chanseokoh deleted the i1556-App-Engine-context-menu-bug branch March 9, 2017 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants