 Good recording started. So this is Mark Wade and Diraj Singh Joda talking on January 17 2022. We're going to do a pair programming session on parameterized parameterized trigon plug-in. So let's look at the shared screen first and we'll just start with this. So can you see the screen okay? And is it's probably better if I give it more a bigger text. Is that font big enough to read? Yes it is. Okay great. All right so then what we need is I need to clone the github repository for parameterized trigger and so parameterized trigger. Okay so I think that's it and now what we're going to do is I'm just going to be lazy like this. Copy that. gh repo clone that. Okay and I'm gonna say gh repo fork because I don't yes and I'd like to add a remote and there we are. Good. All right so now do you already have a pull request pending for something that I should be borrowing from you to get most current things? Yes so for the upstream repo of this plugin there is one PR that I've opened. Okay good so let's look at that. Which one is it? Oh here it is. Update parent palm. Okay all right so let's check out that pull request and it's so the change is you've updated the parent palm from 4.31 to 4.33. Okay now I'm surprised there's not already a PR that's doing that. Interesting okay so yes there is the bump plugin from 4.31233. Oh okay good all right so here's this dependable one. All right good okay and this one has failing checks. Okay so already would you like to investigate the failing checks on this pull request before we go elsewhere or do you want to what would you like to approach first? Yes I think if we investigate the errors in this PR that would be great because that's where I'm confused. Okay all right good let's do that then so and we can we could do that either from your PR or it looks like from the dependable PR because I suspect they're the same change right? Yes they are. Okay good so I'm going to borrow the dependable PR just because it's been there and the machinery should be working and isn't. Let's find out why not so console output says ah this is the upper bound dependency checks okay good so now I should be able to see exactly that. If I do it if we look at my thing here is the update parent palm commit on top of the master branch so I'm going to do a clean skip tests verify and this will take a little bit because it first has to download a big chunk of the internet okay and so I can see the exact failure. That's good that's what we wanted. Now if we look at if we look at the palm file okay so when we were looking at it earlier we said version and this thing is already using so I'm going to do the same steps we had done earlier where we said I'm going to switch it to use 2.289.x and when you did that oh go ahead or was it 2.289.1? Well so in the palm it uses .x and then in the the Jenkins.version value mm it uses an actual version number 2.289.1 okay so what this is is this is a property that's passed into uh or that's used throughout the the project to decide what version specific release of Jenkins the bomb is actually an artifact so it's a project object model and and that artifact is saved with this name so it's by convention that we know that that reference represents 2.289 just because that's its name now I need that version number again and I'm going to cheat and go grab it from another place where I know I've got it so here's the version that I'm using in platform label so let's borrow that okay so let's look at what's changed so far yes so here here are the things that have changed in the palm so far before we even try to compile it the things I took out is I deleted the reference to sub versions version number may or may not work I may have to put it back I deleted promoted builds may or may not have to put it back I deleted the the version for conditional build step and I deleted a waitility and I updated us to a newer version of the palm the of the of the Jenkins version any questions so far no nothing so what do you delete the versions because we will be using bomb which does the work by itself right that's the idea is we're going to lean on someone else who's already doing the heavy lifting the the hard work so that we don't have to do a bunch of computing so let's try to compile again I expect it's going to fail but let's see all right so it fails brutally by saying promoted builds has to have a version number so if we whoops look at this some problems were encountered while processing the palms says dependencies that version for promoted builds jar is missing so the thing that I did to delete it didn't work so I need to switch back and I'm going to borrow my diff tool and make it do the work for me so this one I have to keep let's go back and look what was the other failure the other failure was conditional build step and a waitility so I've got to put both those back as well so let's go back and put those back whoops would help if I didn't kill the buffer I was working on okay so subversion we think we can delete the version number but conditional build stuff we can't a waitility we can't and that's it I'm sorry Ed what did we do for promoted build as well we need to add the version for it as well let's check so subversion we deleted the version number yep so promoted builds it's back okay great good good reminder thank you so let's try it again okay so nope it still says okay and now I deleted the subversion version number and it still uses 2.15.1 right okay so that means my let's use the bomb while a good thing isn't going to fix this I didn't get you can you please repeat it yeah so so the idea we had originally started with was hey let's use the plug-in bill of materials and the plug-in bill of materials didn't solve the problem right right so so now the question is okay what will solve the problem and the web page mentioned hey if you put a reference to this the highest numbered version of these of this thing it should solve it and notice that it's test scoped oh okay so I think well so I think what we could try is we could just try putting a dependency on it now interesting that there's a mojito so so looking at this dirage I'm a little surprised okay we've seen a reference to subversion that I've seen right there's a there's clearly a dependency exactly but oops but I didn't see a reference to mojito exactly there are some things mentioned in this pre which are not there in the pomex okay good all right oh no there it is but there's mojito interesting okay so mojito so what mojito has is it depends on bite buddy and the bite buddy uses this so one one technique that we could use particularly since okay this is where this is where thinking about these things can help me so mojito is one of the things involved subversion we've declared a dependency on mojito as well mojito we didn't declare a version number because we're relying on it coming from Jenkins either Jenkins plug-in bomb or from Jenkins core so one way or the other somebody else is giving us that version number but since it's a test dependency we could put an exclusion to exclude this thing and see if that helps us because the test will then tell us if it failed because we excluded it does does that make any sense to you questions so you're excluding so you found out that jna platform has different versions and one of them is 5.8.0 and it's a test one so you want to avoid this dependency mismatch so that's why you're excluding this one from the formation right that was that was just me thinking allowed what experiment should i try i don't know if it's going to work but i was thinking i want to try an experiment let's see if i could just exclude it and if that would solve if that would get rid of the problem because because it's a test oh because it's a test dependency the test will either pass or fail when i make that exclusion if they pass then it's probably okay if they fail all right the technique didn't work so what i was going to do is take this exclusion copy it whoops in in the real palm not in the fake not in the old copy so let's see that was machito right yes so i was going to add another exclusion saying and now now i admit this is because it's test i'm not sure that if this had been production code we were shipping that i could do this but because it's a test thing i i don't have to worry about it being in production code so you're saying the implication of this can be the test might fail or pass right right exactly the outcome may be that hey i do this may even no longer has the warning but then the test failed catastrophically and if that's the case then it didn't help it's just this experiment then failed yes okay but if if it turns out that it works then then the experiment passed and we can just make it because we're already excluding this thing from machito core the hamcrest okay right hamcrest is a uh a java unit test um what would you call it a java unit test um for literate um assertions okay so that that worked oh oh there are some errors there there are and those errors are spot bugs warnings okay so we've we've got more work to do and this is this is exactly like you would see on on your experience it's okay all i wanted to do was update the parent palm but the act of updating the parent palm required some other things in order to allow it to happen one of them is this is this additional exclusion okay that means the other is spot bugs has more information with the new parent palm than it did with the old and it's now telling us hey here are these problems you need to fix right that makes sense so now we'll be working on these errors that spot book has told us about right well actually and i i described it incorrectly i should be more more more accurate here i suspect the reason these spot bugs messages appeared now and weren't visible before is we changed the jankin's minimum version so instead of compiling with jankin's 2.263 we're now or 2.270 we're now using 2.289 right okay so let's look at the differences again so we removed the subversion versions call out it's still the same but we removed it we don't have to track it anymore yes we added a new exclusion to the mojito core test package now there might actually be a different way to approach this one let's commit this one that we've got now and then we'll investigate to see if there's a different way to do it so this one is exclude required jankin's 2.289.1 instead of 2.270 use the recommended minimum jankin's version and now oftentimes when i'm writing a commit message like this i'll go look at the stats data to see what what the what the statistics are for actual usage of that plugin at various versions so parameterized trigger okay here it is so what this is opening for me is a spreadsheet that shows versions of the plugin installed so across the top it's plugin versions and along the left it's jankin's versions yes now as i hover over this i see that 94 percent of 2.41 installs are on 289.1 or newer so for the last three or four releases the users of this plugin are already upgrading to the 2.289.1 or newer therefore we're not harming them so 2.24 we're just doing we're just updating the minimum version and they've already updated so they've updated this plugins jankin's version in their local copy and we're doing the same in the centralized one that's right you said it exactly okay yes so the stats site shows that the that over 90 percent of installations of the last three versions are already running jankin's 2.289.1 users that are updating the plugin are already running jankin's 2.289.1 so if they choose not to update the plugin they won't update to a new release we deliver anyway if they if they are updating jankin's then there it appears they're also already updating this so the def of this commit consists of all the versions that will be needed for the other ones as well right or just the jankin's version update uh well so let's see what it did so in this commit it removed the version it added that exclusion and updated this and this would you like a better description oh yeah because we are doing lots of other things as well but that's well and what we're doing is the other things we're doing are we're doing because we have to do them in order to make that change but let's describe those so okay so it is exclude exclude hamcrest no exclude gna right exactly from mochito core test dependency to resolve an upper bounds dependency issue okay what else would we want to say about this i think that's it is there more that you would like said in the commit message uh we also created bomb version but do we need to mention that or just leave it good the good thing let's do that so upgrades plug in bomb version to the most recent release for jankin's 2.289.x good yes looks great okay so now we probably better run tests because all we did was check that it compiled successfully let's see about running tests just a minute we have a computer that has multiple cores let's not waste those multiple cores d4 count equal to one one c one and a capital c one c what that is is that says give me one java virtual machine per core on this computer so it's basically use every core on the computer to run tests in parallel wow okay awesome so since a basal grow i think suggested idea to automate this whole process into and wrap it into a software tool which helps us to improve each and every plugin and make sure that they follow same standard in the whole ecosystem right so i was wondering like if i want to automate this exact process like if i or it's easier to update the version of the parent form but it's difficult to solve the issues that come after we update it so i was wondering how should we automatically resolve everything that we are doing right now and and i truly do not know that i think you asked the exact question that i ask about as i look at each of the steps on contributing to open source each of the steps in that document i thought okay which of these could i confidently automate and for instance i could see automating update parent palm but only submit the pull request if a local build succeeds because it's pointless to update the parent palm only to give them a failing build but if the build fails then i've got to do a lot of investigation to understand why did it fail and what else do i need to propose exactly that is a problem like i cannot make it fully automated well and i'm i'm sure that basal basal's expert at software development he's probably got a way to make it automated and could guide that i just don't know i don't know the technique okay that's great so should should i ask him about this oh yes yes absolutely okay on our channels video channels don't know so you could you should be able to reply to the email list where he made the suggestions saying hey i'm interested in this and i'd like to have a further conversation about it okay yes i don't remember that was it was on community.jankins.io exactly yes yeah so what you can do is highlight the block that you've got a question about press the reply button and it will actually quote that and give you the reply and and you can then provide a reply okay so our build our tests pass but there are uh spot bugs issues so we've got to fix those so the the job is not yet complete so let's let's fix those okay so we've got an unused public or protected field in well here let's get those up in a okay so here they are says unused public or protected field so let's look at this public boolean build all nodes with label unused really why would it be unused why would you ever feel that's unused truly it is unused so it should be there at all right should what it should not be there at all right yeah except that now this is where it gets complicated public fields are part of the published api and therefore once a jankins once a jankins plugin has published a release with that public field it will break other people if we delete it so so the but it it absolutely is correct that at least in this class it's not using build all nodes with label and it's searched we searched let's double check our search because i'm just astonished why have it if it's unused there we go okay so it is used in some tests this one sets a value false so this that maps to that field right that's correct okay so i mean if we delete it i think the tests that use those config xml files will break yes so our choices are we could suppress the warning because it's part of the published api and the way or we could what else try we could do all sorts of heroic code things in order to be able to delete it safely for me i think it's simplest just to suppress the warning so we're doing this because spot bug is not happy with this one correct okay yeah so if we look at the build output what does it say it says there's an unused public or protected field and we've looked at it and we agree it is unused it's not being referenced in any way but if we delete it we risk breaking compatibility for existing users and for right tests and other plugins that might be using this plug-ins api so my thought was we should suppress the warning and the way we suppress a warning is if we're lucky we do this at suppress fb warnings and now i have to go find something to copy this is what we're suppressing field is unused but would change the public api if we deleted it now now we've got to go got to go find the the actual what do we import to get this suppress fb warning etc so what what you're experiencing now is how do i handle spot bugs warnings that appear hmm right question so far yes so this um this variable uuf yeah news public or protected field so is this like an id for this or it's like a generic it is it is exactly an id for it's an identifier for that class of error so if i copied this and bring up a web browser to go looking for i don't know that one let's bring up another web browser here we go spot bugs and i look for that here's the bug description it says this field is never used the field is public or protected so perhaps it is intended to be used with classes not seen as part of the analysis if not consider removing it oh so they're like oh i get it now makes sense hmm okay so based on something any kind of problem that we'll be having it in our code spot bug will map to these different id's and will be showing in the output right exactly yeah so spot bugs will tell us hey it has told us there's an unused public field and it's correct there is and this is the it's identifier for the message it gave us and now the question is what do we do with and i think our choice would be let's suppress it in order to suppress it i need to find out how to import this the right thing for suppress fb warning so just a minute let's bring back our web browser again and we're going to search for spot bugs import what to import to use suppress fb warnings and it says it's not helping me i need the import statement in java didn't help me at all okay so we're going to look at some source code then because i happen to know where i can find an example of a suppression like this here it is so all i did is go grab from somebody someplace else what the java import statement is in your ide will probably show you this without you having to do all these things that i'm doing all right it's great it's just in my case i'm not running the ide because i'm using a remote terminal yes your favorite one emacs it's exactly that's i am using that editor okay so i'm going to try to compile again let's see if i was able to silence that one spot bugs warning yes and that did not do it so it may be that i've got a wrong syntax let's go back to that place where we had the examples and maybe because it has value is equal to then the id yeah exactly i think you're right i think we need to use the full syntax let's at least try it yeah because it's a string so it's amazing that it didn't fail to compile so we say value equals this justification equals part of the public api that's used as a way for the the person who is suppressing this warning to communicate to the next reader why they're they're suppressing it did that answer your question yes okay let's try compiling again oh it would help if i use the correct symbol wouldn't it it's plural and there will still be other find bugs warnings but this should get rid of one of them exactly and yes it did the next one is batch condition so let's look for it oh now that's a fun one it's a private field but again notice this thing right here this read resolve deleting even a private field can break compatibility okay so i would tend to suppress this one as well so let's go borrow our oh you might need to change the idea as well oh you're right you are correct thank you for catching that because it's not an unused protected or private field is it yes it's just i think you you have unused if i'm not wrong unused field okay right very good nice catch okay oops and let's and did we import it as well in this file did it's right up here at the top i just did that yes so let's try compiling again okay we're down to two yes this one is high level oh it says and this one will be a fun one because plug-in Hudson plug-ins parameterized trigon plug-in shadows the simple name of the superclass and what that means is we've got a class this long name that inherits from this short name and you can predict that that makes it very difficult to decide when i use short form like this plug-in what do i mean exactly but if we change the class name we've now changed the published api and broken things so we can't do that so now we have to and i think all we can do is go suppress this message because we aren't allowed to as far as i can tell aren't allowed to change that class name otherwise we'll break it exactly right so let's go do that there it is public class plug-in extends Hudson dot plug-in that is terrifying oh the places where things can go wrong with that choice okay it's unambiguous in every use there is no case where it's using the word plug-in unqualified so it actually is safe to suppress this one it's just scary that such a thing exists so can you also show me how is it unambiguous how are we qualifying in the right way so i would expect to find the word plug-in somewhere without a qualifying package name that leads it and so what i did was i looked for the word plug-in okay there's plug-in in the pink and there's a qualified a fully qualified reference to Hudson dot plug-in and here's the only other reference to plug-in and it's fully qualified right Hudson dot plug-in right exactly so as far as i can tell looking at this it looks like it's being unambiguous it's okay even the exactly even though the naming is similar but the usage was done in the right way that's what i think anyway sure is it value or oh yes it is thank you good for you for having a better memory thank you very much and then it's justification yes okay now yeah we need the text for the import statement let's try it again yes okay only one spot bugs warning left so we go looking for batch condition and it's also in so if we go here so comfortable so far okay so that that seems to have worked so what are our changes oh go ahead did you have a question no i was just observing that we don't have any error so um yes so we don't have any other error for anything else right correct okay that's great so we have we have with those four suppressions that we just did we have silenced all the spot bugs warnings now we didn't fix them we just said we've thought about this and and we don't see any problem with how what what's being done so let's look at the changes change number one suppress it change number two suppression suppress it change number three import suppression change number four import suppression okay so far yes okay so then let's put these into a commit message all right so suppress spot bugs warnings analyze the warnings non issues not an issue suppress the warnings okay so what have we done we have required Jenkins 2.2 80 90.1 and suppress spot bugs warnings to allow it to pass yes exactly now one of the things we did not discuss is Makito core let me look and see if I remember correctly oh no okay good this Makito core is the is the primary is the main artifact for Makito so it's okay good all right there's another one called Makito dash all that has many more dependencies in it and and that is not what we need so we excluded jail platform from Makito core so there might be some tests that uh you're saying that it might or it may fail like it might or it might not fail so I was wondering that since we have removed dependency it should fail right no I well if if that dependency if it attempted to use that dependency yes that would fail but I as far as I can tell it didn't actually attempt to use it because if it if it were using it I would expect let's let's run the test again just to be safe just to be sure but if it if it doesn't use it then why is it in the form five as a dependency oh it's it's that it does it may it may use it in a path that we don't execute no test failures yet that's a good sign yes and the computer's got five of its cores busy at once so that's good so it must be 9 20 p.m for you right in colorado it is I'm almost done I actually you have a working day you should be at work at the office today right I mean I'm disrupting you from your work no not at all it's pretty flexible that way but it's I should be the one who's of all the things because it's your time to sleep it's after office hours for you oh yeah I'll I'll definitely go to bed and get some sleep absolutely and I also want to talk to you after the meeting I think today would not be a good idea because it's too late or maybe next week regarding CloudBees discussion yeah let's let's have that so I think I think we're actually almost done here I'm going to go ahead and let's let this finish I'm going to turn off the recording and while we're waiting we can just we can have our conversation great