microsoft / SCXcore

System Center Cross Platform Provider for Operations Manager
Microsoft Public License
36 stars 31 forks source link

Split bundle_skel.sh into bundle_skel_<PLATFORM>.sh #33

Closed kapilash closed 8 years ago

kapilash commented 8 years ago

bundle_skel.sh is split into

Change Linux bundle to install/upgrade all packages as a set

jeffaco commented 8 years ago

Okay, there's lot of remaining cleanup here, particularly for UNIX platforms. I only reviewed the first UNIX file (AIX), assuming you'll make all related changes to the other UNIX files (HP, Solaris). And I want to use your time efficiently. So here's my proposal:

  1. Clean up AIX and all other UNIX files,
  2. Clean up Linux file,

Do a limited test pass. By that, I mean: Do a pbuild, and install on at least one flavor of each platform (AIX 6.1 OR AIX 7.1, don't bother doing both; one HP, one Solaris, and one Linux). Then send me an updated PR. I'll do one final pass and, assuming no remaining problem will sign off.

At that point, assuming no further changes, do a complete test pass (install on all platforms that we support) and commit. If any of the tests fail, then fix that platform and rerun all installation tests for that platform.

The idea here is that, for the final set of changes, you want to install and upgrade on every single platform at least once. But you don't need to do that if you know further changes are coming down the line. And I don't want to catch typos that at least a basic install / upgrade would have caught.

Seem reasonable? If so, do another cleaning pass and send me a new PR, thanks. Hopefully you can get to this quickly; I think we'd both like to wrap this up in the next day or so.

Nicely done, by the way. Having one combined file was a real mess after a while. I think these improvements will make a big difference in maintainability. Imagine just testing on Linux if that's all you changed! :smile:

jeffaco commented 8 years ago

Very nicely done. I wish these changes were in place years ago, it would have made our maintenance work a whole lot easier. You really did do a great job with this, it's so much cleaner and nicer, and it will pay off in spades with easier maintenance down the road.

If you make any further changes, verify your installation tests. If you took shortcuts in prior installation tests, do full testing now (after any final changes). You want to try hard not to commit something broken to master considering we're getting close to shipping the 2016 version.

You have sign-off. I did note that you have two separate commit messages. Please merge this into one meaningful commit message before merging to master. If you need instructions on how to do that, let me know, although I think it's documented (more or less) in ostc-docs.

Basically, assuming that your two commit messages are the two most recent changes in the git log output, then do:

git reset --soft HEAD^
git commit --amend

The git commit --amend should bring up an editor where you can correct your commit message.

The git reset --soft HEAD^ command basically says "undo the last commit, but keep any source changes made in that commit staged". Thus, when you git commit --amend, the reset changes are merged with the prior changes.

kapilash commented 8 years ago

Thanks for all the guidance. I've included the additional comments related to OMI_EXIT_STATUS and the changes needed in the sunos skeleton.

I squashed the commits and did thorough checking :

on all the pbuild platforms (as well as on an RHEL vm).

Linux Comparison Code to SunOs skeleton : I will send these changes in a separate PR.

Removing PROVIDER_ONLY for Linux : I will club this too with the above new PR. I will analyze the impact on make and configure before I do so.