Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 3.0 #16

Merged
merged 8 commits into from
Nov 22, 2018
Merged

Release 3.0 #16

merged 8 commits into from
Nov 22, 2018

Conversation

lagartoflojo
Copy link
Collaborator

@lagartoflojo lagartoflojo commented Nov 20, 2018

Pull Request Checklist

General

  • Update Changelog following the conventions laid out on Keep A Changelog

  • Update README with any necessary configuration snippets

  • Binstubs are created if needed

  • RuboCop passes

  • Existing tests pass

Purpose

  • Cut a new version of the gem.
  • Does not change, add or remove any functionality from the previous version.
  • Updated instructions to get chat_id
  • Update link to Changelog guidelines
  • Update sensu-plugins to ~> 2.7, which includes breaking changes
  • Update Rubocop to latest, and fix violations
  • Fix strictness of rest-client gem dependency
  • Remove obsolete Gemnasium badge
  • Fix typo in 2.0.1 changelog header
  • Remove support for Ruby < 2.3 per the Sensu Plugins policy

@lagartoflojo
Copy link
Collaborator Author

@majormoses if you have some time, I'd appreciate your input on this one :)

@majormoses
Copy link
Member

Copy link
Member

@majormoses majormoses left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good though I have some feedback. Apologies if I have not spent enough time either help documenting processes or go over them with you. Also I will be traveling tomorrow for the holidays and probably won't respond until Saturday.

.travis.yml Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
sensu-plugins-telegram.gemspec Outdated Show resolved Hide resolved
sensu-plugins-telegram.gemspec Show resolved Hide resolved
sensu-plugins-telegram.gemspec Show resolved Hide resolved
sensu-plugins-telegram.gemspec Outdated Show resolved Hide resolved
CHANGELOG.md Outdated

## [Unreleased]

## [1.0.1] - 2017-07-29
## [3.0.0] - 2018-11-20
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will probably need to adjust the date when you have addressed my comments.

Typically we do the changes in a branch and leave everything under ## [Unreleased] without touching the version file. There are several reasons we update the versions and dates post acceptance of the change:

  • PRs are not a FIFO queue, as such versioning them prior to acceptance leads to needing to ask people to rebase more often
  • we might disagree on impact of change (in this case we don't)
  • we might choose to lump several PR changes into a single release to reduce the number of rebases we ask users to make when they are touching the same files.
  • we date the release rather then when the contribution was submitted, PR review and acceptance has no SLA and is not guaranteed to be merged within the same day

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for the explanation!

CHANGELOG.md Outdated Show resolved Hide resolved
gem build was raising a warning.
Due to the new [Telegram API privacy
mode](https://core.telegram.org/bots#privacy-mode), the `getUpdates`
call does not show all messages sent in the group. Instead, the bot only
sees a limited set of messages. Sending any `/cmd` message and also
mentioning the bot will make the message appear in `getUpdates`.
Closes #10.
The occurrences filtering was removed from sensu-plugin and moved into a
Sensu extension (https://github.com/sensu/sensu-extensions-occurrences).
The example configuration has been updated.
@lagartoflojo
Copy link
Collaborator Author

Thanks a lot for the review!

Who should remove the "Status: Awaiting Response" label once I've responded? :)

@majormoses
Copy link
Member

Who should remove the "Status: Awaiting Response" label once I've responded? :)

In most cases since users don't have the ability to remove the label whoever was the initial reviewer should but since you have access you can remove it.

Copy link
Member

@majormoses majormoses left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@majormoses majormoses merged commit 2ab6341 into master Nov 22, 2018
@majormoses
Copy link
Member

I will reply back with a link to any commits/changes that need to be made prior to release and the release process so you can do this in the future.

@majormoses
Copy link
Member

Looks like the only thing that was missing was updating the diff links: 6c99cbd

@majormoses
Copy link
Member

So the process looks like this:

  1. update changelog, versions, and if applicable readme
  2. commit them with something like this:
 $ git commit -am 'prep for 3.0.0 release'
 [master 6c99cbd] prep for 3.0.0 release
 1 file changed, 2 insertions(+), 1 deletion(-)
  1. push the change to master:
$ git push 
warning: not sending a push certificate since the receiving end does not support --signed push
Counting objects: 3, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 986 bytes | 986.00 KiB/s, done.
Total 3 (delta 2), reused 0 (delta 0)
remote: Resolving deltas: 100% (2/2), completed with 2 local objects.
reate 3.0.0To github.com:sensu-plugins/sensu-plugins-telegram.git
 2ab6341..6c99cbd  master -> master
  1. create the actual release, if tests pass it will automatically push to rubygems:
$ hub release create 3.0.0

https://github.com/sensu-plugins/sensu-plugins-telegram/releases/tag/3.0.0

https://travis-ci.org/sensu-plugins/sensu-plugins-telegram/builds/458202673

@majormoses
Copy link
Member

I will check back to see later if it built as it looks like we have clogged up our travis builds for the moment. While its not required I generally like to add a link to the actual release published to rubygems. The main reason for this is that it lets people know that after going through the review process their change has been released/published in a timely manner.

@majormoses
Copy link
Member

@majormoses
Copy link
Member

My general rule is that except in extreme cases releases should be cut within 24 hours of acceptance of a PR. Generally I do it immediately after but sometimes for example if travis is having issues we need to delay it.

@lagartoflojo
Copy link
Collaborator Author

Excellent, thanks a lot for the quick replies and information! I thought we'd have to wait till Saturday 😉

@lagartoflojo lagartoflojo deleted the release-3.0 branch November 22, 2018 08:42
@majormoses
Copy link
Member

@lagartoflojo your welcome I wanted to set the expectation since I had no idea what the plans with the family were so was unsure what kind of downtime I would have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants