Add link to commit on GitHub when closing a fixed bug/added feature

site feedback
1

When archiving a topic on Meta for closing out a bug/feature I think it would be helpful if we linked to the actual commit message on GitHub.

6 Likes
2

Yes, You can find some bugs here that did receive the right treatment, and it should be encouraged.

4 Likes
3

This hasn’t received the attention it should. I really think that the dev team should make this a standard of theirs so that everyone can see when a fix has actually been committed.

2 Likes
4

In general we add links to commit when fixing bugs, longer term I may look at extending the github plugin to do this automatically.

2 Likes
5

Doesn’t really seem like it. I found three examples of fixed bugs from the past 24 hours where there was no link to the relevant github commit:

Looking over bugs closed in the past day, it seems @zogstrip and @elberet post the links for their bugs, but the other members of your team don’t.

1 Like
6

When it is my bug and I fix it I generally add a link. I do not when I am closing something that got fixed ages ago and digging up the commit is impossible.

1 Like
7

Just out of curiosity: if it had been fixed ages ago, why was the bug still open? I usually close mine as soon as possible to get them off my todo list in Bugzilla (still 149 and counting on it, though).

1 Like
8

The general reason I see for old stuff that gets missed is “fixed as a side-effect” (of a redesign, refactor, other work, accident, etc)

9

That could explain one of the examples I provided, which was opened on May 21. But it doesn’t explain the other two, which where opened 1 and 6 days ago. Based on comments, those two were clearly current issues, not ones that were fixed and forgotten.

10

Oh really?

Bug report:

Topic where you reported a fix:

Not only did you not post a link to the commit that fixed the issue, but you reported the fix in a new topic, independent of the original bug report, and the new topic was in the meta category, instead of in the bug category.

11

To be fair, his topic was made before the bug was reported… By hours!

Plus, since it is badge related, the fix was likely running a query to strip the badge that was accidentally handed out. It might not have actually required a code change.

1 Like
12

Ok, I’ll give you that. I didn’t notice because of the way timestamps are masked on posts. However, @sam still could have pointed the bug report to the topic in meta.d. Instead, he just said that he wasn’t going to look into it unless it happened on other forums.

This probably isn’t all that was involved. Or at least, if this was the fix, it wasn’t sufficient. As you pointed out, @sam’s topic in the meta category was closed before the bug topic was opened. The two appear to be talking about the same bug, so did @sam really fix the bug if it was reported again almost 12 hours later?

13

He had to perform a cleanup which didn’t quite clean everything up (based on what I read of the 3-4 different topics posted about the subject).

The way he phrased it, made be believe he was either 1) altering the Badge SQL Query in the Admin area and then re-running the Trigger Badge job, or 2) ran a delete/update statement to remove the badges that were handed out.

So maybe there was code involved, but I didn’t get that impression from what I’ve read.

14

3 points:

  1. I would count SQL as code
  2. This appears to have affected default badges. I certainly hope that the SQL for those badges is in the code repository.
  3. If the trigger SQL was faulty, then just deleting the bad badges would not actually fix the issue. The underlying SQL would need to be fixed.
15

Although it likely exists in the code somewhere, I don’t know to what extent. You can actually create/alter badges within the Discourse Admin area, so he may have been testing his badge before actually implementing it (yeah, long shot – but it could have happened). So altering it in the admin area and then re-running the job would/should have resolved it (I think).

However, this seems to indicate otherwise.
https://github.com/discourse/discourse/commit/42104685f7fbe3c80870d75fedb298fb3f29a67e

Coupled with the prior checkin of
https://github.com/discourse/discourse/commit/7f3797b6352456b2f70b28d71c6d5cdd404f9d5e

1 Like
16

I don’t think we need to blame people for not posting their link to the github commit. Until there is a discourse github issue tracker plugin this is an impossible problem to solve.

In regards to this issue that was posted above:

https://meta.discourse.org/t/images-in-the-welcome-email-load-over-http-breaking-ssl/19905/7

I think whoever closes the topic should post the link to the commit on github or kindly remind the person who made the actual commit to post the link before closing the topic.

If the topic isn’t closed maybe someone can volunteer and post the link and help out the community.

1 Like
17

There is no requirement to link to GitHub when closing an issue.