Code review responses

core/src/main/java/hudson/model/Job.java

@@ -1075,14 +1075,14 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
                 scmNames.add(s.getDescriptor().getDisplayName());
             }
             scmDisplayName = " " + Util.join(scmNames, ", ");
+        }
 
-            for (RunT r = getLastBuild(); r != null; r = r.getPreviousBuild()) {
-                int idx = 0;
-                if (r instanceof RunWithSCMMixIn.RunWithSCM) {
-                    for (ChangeLogSet<? extends ChangeLogSet.Entry> c : ((RunWithSCMMixIn.RunWithSCM<?,?>) r).getChangeSets()) {
-                        for (ChangeLogSet.Entry e : c) {
-                            entries.add(new FeedItem(e, idx++));
-                        }
+        for (RunT r = getLastBuild(); r != null; r = r.getPreviousBuild()) {
+            int idx = 0;
+            if (r instanceof RunWithSCMMixIn.RunWithSCM) {
+                for (ChangeLogSet<? extends ChangeLogSet.Entry> c : ((RunWithSCMMixIn.RunWithSCM<?,?>) r).getChangeSets()) {
+                    for (ChangeLogSet.Entry e : c) {
+                        entries.add(new FeedItem(e, idx++));
                     }
                 }
             }

core/src/main/java/hudson/model/View.java

@@ -116,7 +116,6 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
-import static jenkins.model.Jenkins.checkGoodName;
 import static jenkins.scm.RunWithSCMMixIn.*;
 
 import org.kohsuke.accmod.Restricted;
@@ -253,7 +252,7 @@ public abstract class View extends AbstractModelObject implements AccessControll
      */
     public void rename(String newName) throws Failure, FormException {
         if(name.equals(newName))    return; // noop
-        checkGoodName(newName);
+        Jenkins.checkGoodName(newName);
         if(owner.getView(newName)!=null)
             throw new FormException(Messages.Hudson_ViewAlreadyExists(newName),"name");
         String oldName = name;
@@ -730,23 +729,21 @@ public abstract class View extends AbstractModelObject implements AccessControll
             Map<User,UserInfo> users = new HashMap<User,UserInfo>();
             for (Item item : items) {
                 for (Job<?, ?> job : item.getAllJobs()) {
-                    if (job instanceof SCMTriggerItem) {
-                        RunList<? extends Run<?, ?>> runs = job.getBuilds();
-                        for (Run<?, ?> r : runs) {
-                            if (r instanceof RunWithSCM) {
-                                RunWithSCM<?,?> runWithSCM = (RunWithSCM<?,?>) r;
+                    RunList<? extends Run<?, ?>> runs = job.getBuilds();
+                    for (Run<?, ?> r : runs) {
+                        if (r instanceof RunWithSCM) {
+                            RunWithSCM<?,?> runWithSCM = (RunWithSCM<?,?>) r;
 
-                                for (ChangeLogSet<? extends Entry> c : runWithSCM.getChangeSets()) {
-                                    for (Entry entry : c) {
-                                        User user = entry.getAuthor();
+                            for (ChangeLogSet<? extends Entry> c : runWithSCM.getChangeSets()) {
+                                for (Entry entry : c) {
+                                    User user = entry.getAuthor();
 
-                                        UserInfo info = users.get(user);
-                                        if (info == null)
-                                            users.put(user, new UserInfo(user, job, r.getTimestamp()));
-                                        else if (info.getLastChange().before(r.getTimestamp())) {
-                                            info.project = job;
-                                            info.lastChange = r.getTimestamp();
-                                        }
+                                    UserInfo info = users.get(user);
+                                    if (info == null)
+                                        users.put(user, new UserInfo(user, job, r.getTimestamp()));
+                                    else if (info.getLastChange().before(r.getTimestamp())) {
+                                        info.project = job;
+                                        info.lastChange = r.getTimestamp();
                                     }
                                 }
                             }
@@ -1055,7 +1052,7 @@ public abstract class View extends AbstractModelObject implements AccessControll
         }
 
         try {
-            checkGoodName(value);
+            Jenkins.checkGoodName(value);
             value = value.trim(); // why trim *after* checkGoodName? not sure, but ItemGroupMixIn.createTopLevelItem does the same
             Jenkins.getInstance().getProjectNamingStrategy().checkName(value);
         } catch (Failure e) {
@@ -1308,7 +1305,7 @@ public abstract class View extends AbstractModelObject implements AccessControll
                         || requestContentType.startsWith("text/xml"));
 
         String name = req.getParameter("name");
-        checkGoodName(name);
+        Jenkins.checkGoodName(name);
         if(owner.getView(name)!=null)
             throw new Failure(Messages.Hudson_ViewAlreadyExists(name));
 
@@ -1370,7 +1367,7 @@ public abstract class View extends AbstractModelObject implements AccessControll
         try (InputStream in = new BufferedInputStream(xml)) {
             View v = (View) Jenkins.XSTREAM.fromXML(in);
             if (name != null) v.name = name;
-            checkGoodName(v.name);
+            Jenkins.checkGoodName(v.name);
             return v;
         } catch(StreamException|ConversionException|Error e) {// mostly reflection errors
             throw new IOException("Unable to read",e);

core/src/main/java/hudson/scm/SCM.java

@@ -526,8 +526,10 @@ public abstract class SCM implements Describable<SCM>, ExtensionPoint {
      * can be before/after your checkout method is invoked. So if you are going to provide information about
      * check out (like SVN revision number that was checked out), be prepared for the possibility that the
      * check out hasn't happened yet.
+     *
+     * @since FIXME
      */
-    public void buildEnvVars(@Nonnull Run<?,?> build, @Nonnull Map<String,String> env) {
+    public void buildEnvironment(@Nonnull Run<?,?> build, @Nonnull Map<String,String> env) {
         if (build instanceof AbstractBuild) {
             buildEnvVars((AbstractBuild)build, env);
         }
@@ -535,8 +537,8 @@ public abstract class SCM implements Describable<SCM>, ExtensionPoint {
 
     @Deprecated
     public void buildEnvVars(AbstractBuild<?,?> build, Map<String, String> env) {
-        if (Util.isOverridden(SCM.class, getClass(), "buildEnvVars", Run.class, Map.class)) {
-            buildEnvVars((Run) build, env);
+        if (Util.isOverridden(SCM.class, getClass(), "buildEnvironment", Run.class, Map.class)) {
+            buildEnvironment(build, env);
         }
         // default implementation is noop.
     }