search

bgamari / trac-to-remarkup

Moved to GitLab: https://gitlab.haskell.org/bgamari/trac-to-remarkup
https://gitlab.haskell.org
BSD 3-Clause "New" or "Revised" License
2 stars 0 forks source link

Fix event times #10

Closed bgamari closed 5 years ago

bgamari commented 5 years ago

Ticket creation and closure times aren't set properly.

Appears to be an upstream issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/53279

tdammers commented 5 years ago

https://gitlab.com/gitlab-org/gitlab-ce/issues/46980, apparently.

We may have to patch this ourselves, no?

mpickering commented 5 years ago

I think that was Ben's intent. Should be as simple as adding an extra parameter to the endpoint and threading it through appropriately?

tdammers commented 5 years ago

Guess I'll have to brush up my barely existing Ruby skills then...

mpickering commented 5 years ago

This is the code path for closing an issue.

  1. API method endpoint is defined in lib/api/issues.rb and calls Issues::UpdateService to update an issue.
  2. Issues::UpdateService is defined in app/services/issues/update_service.rb, this then calls update.
  3. update is defined in app/services/issuable_base_service.rb calls change_state
  4. change_state calls close_service.new
  5. app/services/issues/close_service.rb calls change_status
  6. Note is created in app/services/system_note_service.rb in change_status.
bgamari commented 5 years ago

Note that I started pushing through the change here. However, since starting this patch I have spoken with GitLab and they have said they may be able to prioritize the issue. Unfortunately, even if they can it likely won't happen before the 18th so we have two options:

  1. Push the switch-over date back
  2. Push the GitLab fix through ourselves
tdammers commented 5 years ago

If we're going to go for hosted gitlab, then we don't really have a choice, do we? We can push the fix, but unless it makes it into the hosted instance before the 15th, we still can't import. If we were to self-host, then we could of course just apply the patch and then wait for upstream to catch up.

mpickering commented 5 years ago

I thought we were going to self-host? I think this issue is really important to fix before doing the migration as it makes the overall quality much better.

tdammers commented 5 years ago

Yes we are, I had missed that decision.

I agree that this needs to be fixed; with incorrect timestamps, much of the data is going to be worthless.

Ben has half a patch for this, but the question is how much work we should sink into this ourselves at this point; the idea from today's GHC meeting was to first try and extract some commitment from the gitlab team.

On Tue, Dec 11, 2018, 17:52 Matthew Pickering <notifications@github.com wrote:

I thought we were going to self-host? I think this issue is really important to fix before doing the migration as it makes the overall quality much better.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bgamari/trac-to-remarkup/issues/10#issuecomment-446275985, or mute the thread https://github.com/notifications/unsubscribe-auth/ABXjx1ajlr0MYRAtON9fUlR365pnGjNqks5u3-LRgaJpZM4Y-un8 .

mpickering commented 5 years ago

Partial MR is here -https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/23713

bgamari commented 5 years ago

I am using the finished version of this patch in the import.