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

Badge with changes waiting for deployment in README #1605

Merged
merged 1 commit into from
Mar 30, 2018
Merged

Badge with changes waiting for deployment in README #1605

merged 1 commit into from
Mar 30, 2018

Conversation

platan
Copy link
Member

@platan platan commented Mar 27, 2018

Currently we have a "last deployed" badge in README which shows date of the last deployment and it links to https://github.com/badges/shields/commits/gh-pages branch (the gh-pages branch contains changes deployed to production).

This PR replace this badge with a "commits to be deployed" which shows number of commits waiting for deployment to production (number of commits between the gh-pages and the master) and it links to gh-pages...master with a diff between branches.

Use "View" button to see rendered README (or use this link https://github.com/platan/shields/blob/6dde98d77f9444e00684f4e2c3a85a0e5cb34db1/README.md)

Why this change? I often want to check if some change is already deployed to production. This badge helps in this case.

What do you think about this badge? Do you think it is useful?

Alternatively we can leave the "last deployed" badge. The name "commits to be deployed" can be changes to "waiting for deployment"/"commits waiting for deployment".

@shields-ci
Copy link

Messages
📖

✨ Thanks for your contribution to Shields, @platan!

📖

Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS

@paulmelnikow
Copy link
Member

This is awesome!

Something else that might be really helpful is to post a comment at the end of a PR thread, which indicates whether it's been deployed yet. Once a PR is merged we know the commit. I think using the compare API we can check whether a particular commit has been merged into our deploy branch. Though I don't think any of the existing badges could show this information – it seems like we'd need a new one.

@paulmelnikow paulmelnikow added the developer-experience Dev tooling, test framework, and CI label Mar 27, 2018
@platan
Copy link
Member Author

platan commented Mar 27, 2018

Preparing the new one badge should be easy ;-) But how can we automatically add a comment to PR after a merge? Any ideas?

@chris48s
Copy link
Member

But how can we automatically add a comment to PR after a merge? Any ideas?

Have a look at Probot:

https://probot.github.io/
https://github.com/probot/probot

@paulmelnikow
Copy link
Member

I opened #1616 with a design!

@platan
Copy link
Member Author

platan commented Mar 28, 2018

Thank you @chris48s! Probot looks promising and I will try to create something using it.

@platan platan merged commit 0d5b48b into badges:master Mar 30, 2018
@platan
Copy link
Member Author

platan commented May 22, 2018

I started to work on pasting a badge with a merge status after PR merge using probot. But now I came up with a new idea and I would like to get your opinion.

  • Approach 1
    After a merge probot app adds a comment to merged PR with a badge described in above comments (and here Github commit merge status in branch #1616). If you want to know if your PR was deployed you have to check this badge in comment (every day/week).
  • Approach 2
    After a deployment to production in all pull requests recently deployed we add a comments with an info that PR is deployed. This comments can be added automatically using probot app (watches for changes in the gh-pages branch) or manually using some script. In this approach all users involved in specific PR are notified by Github.

@chris48s @PyvesB @RedSparr0w @paulmelnikow What do you think about these approaches? Which one is better (more user friendly) in your opinion?

@chris48s
Copy link
Member

👍 on option 2 from me. I think its more useful to contributors if they get a notification instead of having to check back.

@RedSparr0w
Copy link
Member

👍 for option 2 for me too.
Will definitely be useful for the PR creators, Although it will create a lot of notifications for us when all the changes have been deployed.

Would it also be worth @ing people from the issue that the PR closes (maybe via an opt in link)?
or also commenting on the issue?
As those people will generally also be waiting for the fix to go live.

@PyvesB
Copy link
Member

PyvesB commented May 22, 2018

I personally prefer option 1. Using badges to provide useful information within the badges/shields project itself is really neat in my opinion. It proves that we trust our own software and showcases a original way of using it.

A lot of people are one-time contributors, who will wonder whether their commit has been deployed simply because they aren't familiar with our lifecycle or don't know how our gh-pages branch works. Solution 2 will not prevent them from asking us about whether their PR has been deployed whereas solution 1 will give them that piece of information without our intervention.

Also, if users aren't aware that solution 2 is implemented or have muted their notifications, they will check back anyway. In this case solution 1 provides additional information. Having to check back is anyway not a bad thing in my opinion, developers might look around at the open issues at the same time or come up with some new ideas by viewing recent contributions. 👍

@chris48s
Copy link
Member

Solution 2 will not prevent them from asking us about whether their PR has been deployed whereas solution 1 will give them that piece of information without our intervention.

This is a very good point. You're right that providing some messaging about what's going to happen at merge/review time is important for a first-time contributor who does not know to expect the notification. 🤔

@platan
Copy link
Member Author

platan commented May 23, 2018

Thank you for your quick responses. I really like @PyvesB's reasoning. In my opinion we can start with the approach 1.

I think we need 2 things for a complete solution:

I'm currently working on the second element - a probot app.

@platan platan mentioned this pull request May 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Dev tooling, test framework, and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants