vaadin / eclipse-plugin

Vaadin Plugin for Eclipse
https://vaadin.com/eclipse
16 stars 8 forks source link

Remove Vaadin 6 scheduled nightly build checks #722

Closed hesara closed 7 years ago

hesara commented 7 years ago

This change removes related code, but does no other cleanup.


This change is Reviewable

ahie commented 7 years ago

Reviewed 25 of 25 files at r1. Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.


[com.vaadin.integration.eclipse/src/com/vaadin/integration/eclipse/IVaadinFacetInstallDataModelProperties.java, line 33 at r1](https://reviewable.io:443/reviews/vaadin/eclipse-plugin/722#-KW2fUonMHzR9KP5nR2Z:-KW2fUonMHzR9KP5nR2:brvnn9v) (raw file):_

    public static final String VAADIN_PROJECT_TYPE = "IVaadinFacetInstallDataModelProperties.VAADIN_PROJECT_TYPE"; //$NON-NLS-1$

    // public static final String USE_LATEST_NIGHTLY = "IVaadinFacetInstallDataModelProperties.USE_LATEST_NIGHTLY"; //$NON-NLS-1$

Why not remove this as well?


com.vaadin.integration.eclipse/src/com/vaadin/integration/eclipse/notifications/jobs/nightly/Messages.java, line 1 at r1 (raw file):

package com.vaadin.integration.eclipse.notifications.jobs.nightly;

Could consider also renaming the package and classes within.


com.vaadin.integration.eclipse/src/com/vaadin/integration/eclipse/notifications/jobs/nightly/NightlyCheckRule.java, line 19 at r1 (raw file):

    }

    public boolean contains(ISchedulingRule rule) {

Nit: missing overrides annotation, also isConflicting. The same also applies to NightlyCheckSchedulerJob. Code cleanup with the framework project's configuration could be run on the whole project?


Comments from Reviewable

hesara commented 7 years ago

Review status: 24 of 25 files reviewed at latest revision, 3 unresolved discussions.


com.vaadin.integration.eclipse/src/com/vaadin/integration/eclipse/IVaadinFacetInstallDataModelProperties.java, line 33 at r1 (raw file):

Previously, ahie (Aleksi Hietanen) wrote… > Why not remove this as well? >

Done.


_com.vaadin.integration.eclipse/src/com/vaadin/integration/eclipse/notifications/jobs/nightly/Messages.java, line 1 at r1 (raw file):_

Previously, ahie (Aleksi Hietanen) wrote… > Could consider also renaming the package and classes within. >

Can consider doing this separately later, I would not mix it in here to make this patch even bigger


com.vaadin.integration.eclipse/src/com/vaadin/integration/eclipse/notifications/jobs/nightly/NightlyCheckRule.java, line 19 at r1 (raw file):

Previously, ahie (Aleksi Hietanen) wrote… > Nit: missing overrides annotation, also isConflicting. The same also applies to NightlyCheckSchedulerJob. Code cleanup with the framework project's configuration could be run on the whole project? >

could run larger scale cleanup separately later


Comments from Reviewable

ahie commented 7 years ago
:lgtm:

Reviewed 1 of 1 files at r2. Review status: all files reviewed at latest revision, all discussions resolved.


_Comments from Reviewable_