Closed lm-ydubler closed 2 years ago
lm-ydubler
commented
2 years ago @armab
Done, done, done, done, and done.
Thank you kindly and please touch back as required.
cognifloyd
commented
2 years ago Please merge in master. We just merged fixes for CI so it will give you real feedback.
cognifloyd
commented
2 years ago OK. CI is now giving real feedback. Everything needs to pass before we can ~merge this~ bootstrap the new pack repo.
lm-ydubler
commented
2 years ago @cognifloyd Thank you for the feedback but will not be making changes to the pack at this time due to formal overhead required with every change. Currently just trying to get it into the Exchange. Do you need me to do anything (not code related) to get the pack into the Exchange?
You made the comment above "Please merge in master. We just merged fixes for CI so it will give you real feedback." Do I have to act on this?
cognifloyd
commented
2 years ago You made the comment above "Please merge in master. We just merged fixes for CI so it will give you real feedback." Do I have to act on this?
I merged in master, so this is done.
Thank you for the feedback but will not be making changes to the pack at this time due to formal overhead required with every change. Currently just trying to get it into the Exchange. Do you need me to do anything (not code related) to get the pack into the Exchange?
The CI tests are failing, so you need to fix the code. We won't add a pack to the exchange where CI tests are failing.
cognifloyd
commented
2 years ago Again, I'm sorry the CI has been broken for so long. Had it been working you would have had this feedback as soon as you created the initial PR. Thank you for helping us get it up and running again! I hope you don't have to jump through too much "formal overhead" to resolve the linting issues.
lm-ydubler
commented
2 years ago It is what it is, glad you're operational again. Glad to have been able to help and thanks for all of your help getting the pack into the Exchange.
I will have to test the pack after making the code changes required to get it into the Exchange. And I will have to make a PR in LM to update our version of the pack.
Right now, updating code to get it into the Exchange...
lm-ydubler
commented
2 years ago I am rushing to get the pack into the exchange but that is not really wise behavior because haste makes waste. My weekend starts tomorrow so I will be back on Monday to make sure all the changes we make to the Pack are tested and go through formal channels before it is uploaded to the Exchange. I will redo some changes such as using multiline strings (as you pointed out) in sensor_constants.py.
Do the scripts automatically add the pack to the Exchange or do I have to confirm it? Thanks again
cognifloyd
commented
2 years ago I marked the PR as a draft. We won't add the pack until you click "ready for review", and a TSC member approves it (prob me). And, no, creating the pack repo is not automated yet. I'm working on that, but I don't expect to use the automation to add this pack.
PS all the linters are green :) So, let me know when you're ready for me to take another look.
arm4b
commented
2 years ago @lm-ydubler I've created a new LogicMonitor Github group and invited you there so you could be a maintainer for the future pack. For the other members, I'll need their existing github usernames, rather than emails. Also, https://github.com/logicmonitor is an organization and not a user.
P.S. The pack looks good to me, License, Readme, Icon. Thanks for the changes :+1: @cognifloyd go for it when you think it's ready, also great coordination work with @lm-ydubler
lm-ydubler
commented
2 years ago @cognifloyd @armab I'll be testing the pack today one last time to make sure all the changes are functional before pushing the "Ready for review" button. I'll alert you when testing has completed. Thanks again!
lm-ydubler
commented
2 years ago @cognifloyd @armab The pack has been tested successfully and can be merged into the Exchange.
If we can add maintainers to the pack after it gets pushed to the Exchange, you can list me as the only maintainer of the pack for the time being with the intention of adding others as time progresses.
Thanks so much, please keep me updated as required.
cognifloyd
commented
2 years ago https://github.com/StackStorm-Exchange/stackstorm-logicmonitor https://exchange.stackstorm.org/#logicmonitor
This has been published to the exchange. :tada:
arm4b
commented
2 years ago Thanks all!
We announced it in StackStorm channels for better outreach:
@lm-ydubler If @LogicMonitor could re-share it in Twitter/LinkedIn with their community, - that would be great. If you guys want to announce the StackStorm integration somewhere in your engineering blog (https://www.logicmonitor.com/blog), - that would be even better!
Hi all, Thanks for running this incubator and I'm glad to have helped @cognifloyd to get it back online.
This is the LogicMonitor (https://www.logicmonitor.com) Pack for StackStorm and we're hoping to graduate it into the StackStorm Exchange.
That said, the owned of this pack is the "LogicMonitor" account found at this url: https://github.com/logicmonitor . Therefore, I am asking that when this pack graduates into the Exchange (for legal purposes):
My personal account that created this PR ("lm-ydubler") loses all privileges and connection to the LogicMonitor pack, and,
The LogicMonitor Github Account named "logicmonitor" or "LogicMonitor" found here becomes the official owner and administrator of the pack and gains all privileges to the pack and it's repository so that it can create pull requests to update the pack, remove the pack, etc. Obviously, this would be within the boundaries and context of however you guys manage packs in the Exchange.
Please keep me in the loop and let me know if I have to do anything in this process. Thanks and best and Merry Christmas! Yuri