I'm happy to do the refactoring if we think that internal consistency is more important

Consistency is important! The checkin vs check_in (or checkIn) inconsistency was already there when I started the PHP implementation and I (mistakenly) didn't think about correcting it. I just followed what was already there and then applied the same naming convention to the JavaScript implementation. I will rename all code from checkin to checkIn (we use camelCase in PHP and JavaScript) and apply similar convention to filenames (either check-in or check_in).

Also, not sure if this should be honeybadger checkins sync or honeybadger sync checkins instead and I didn't yet read the docs on how to build subcommands with Thor.

I had the same dilemma. I ended up following shalvah's suggestion on this structure: honeybadger DOMAIN ACTION

@subzero10 I've renamed everything, praised be ye global rename feature of the editors. I've also added the tabular output of the sync results which was missing. I'm planning on doing a couple of manual tests this afternoon to be sure I am not fooled by all the mocking and stubbing.

I think you mentioned changing the signature of the actual check_in method so that it allows names instead of the id, but I'd love to do that in a separate PR if that's okay, this is big enough as is.

I think you mentioned changing the signature of the actual check_in method so that it allows names instead of the id, but I'd love to do that in a separate PR if that's okay, this is big enough as is.

OK, that works! Btw, it should allow both name or id (i.e it should not break any existing behavior). Here's how it was done for the JS implementation.

@subzero10 PTAL

@halfbyte Reminder that we also have to do the following when this PR is merged:

• change the signature of the actual check_in method so that it allows names or ids

* change the signature of the actual check_in method so that it allows names or ids

See #509

Should I wait for @joshuap's review? I think so?

Should I wait for @joshuap's review? I think so?

Yup :)

Another random thought I just had: I see we're using the project_id from the check_ins config everywhere.

As you said, the project_id is necessary today, because we have to specify which project we are creating check-ins for.
In my opinion, the ideal solution would be to be able to use the project api key and skip the project_id entirely, maybe even the personal auth token.

Have you considered defaulting to the project api_key if people exclude the project_id?

Is this possible with the current API?

We'd have to either invent a way to fetch the real project_id to use for substitution

I think I like this idea.
We could:

• have a route that that takes a project api key and returns a project id (route would be authenticated with the personal auth token)
• drop the project_id field entirely from each check-in

(and most people will use this with a single project)

True. If we come to a solution with what you suggest above, we'll lose this multi-project check-ins possibility, but that's something I'm perfectly fine with.

Update: Another idea I had in mind was to allow setting project_id at the root of the config (next to the api_key) and use it for all check-ins, since we know that will most people will use check-ins with a single project.

Update: Another idea I had in mind was to allow setting project_id at the root of the config (next to the api_key) and use it for all check-ins, since we know that will most people will use check-ins with a single project.

This crossed my mind too! It might be nice if you could set a top-level default that you could optionally override in each check-in. I also like the idea of not having to specify the project id (and personal access token?) at all, though. That said, I don't think we can grant any sort of read access to the project api_key because it's exposed publicly by client-side libraries, like honeybadger-js.

Have you considered defaulting to the project api_key if people exclude the project_id?

Is this possible with the current API?

Nope, it's not possible today, afaik.

This crossed my mind too! It might be nice if you could set a top-level default that you could optionally override in each check-in. I also like the idea of not having to specify the project id (and personal access token?) at all, though. That said, I don't think we can grant any sort of read access to the project api_key because it's exposed publicly by client-side libraries, like honeybadger-js.

😄 Heh, I was so close to doing exactly this (top-level default project-id) but I decided not to in order to get the feature out sooner.

That said, I don't think we can grant any sort of read access to the project api_key because it's exposed publicly by client-side libraries, like honeybadger-js.

👍 Good point, I guess the personal auth token needs to stay.

I also like the idea of not having to specify the project id

Do you think we can go ahead with the following?

• Have a route that that takes a project api key and returns a project id (route would be authenticated with the personal auth token).
  If yes, we won't need to the top-level default project_id either.

@subzero10 @joshuap I think I followed your conversation about the project_id thing but I am not super sure if I can draw any conclusions regarding this PR from it right now? :)

This crossed my mind too! It might be nice if you could set a top-level default that you could optionally override in each check-in. I also like the idea of not having to specify the project id (and personal access token?) at all, though. That said, I don't think we can grant any sort of read access to the project api_key because it's exposed publicly by client-side libraries, like honeybadger-js.

😄 Heh, I was so close to doing exactly this (top-level default project-id) but I decided not to in order to get the feature out sooner.

That said, I don't think we can grant any sort of read access to the project api_key because it's exposed publicly by client-side libraries, like honeybadger-js.

👍 Good point, I guess the personal auth token needs to stay.

I also like the idea of not having to specify the project id

Do you think we can go ahead with the following?

• Have a route that that takes a project api key and returns a project id (route would be authenticated with the personal auth token).
  If yes, we won't need to the top-level default project_id either.

I think this is OK for syncing things, and is a nicer default experience. Since we already have the per-check-in project_id option in this PR, is there any harm in leaving it as an override if some people want to sync check-ins to other projects?

I'm less certain about performing check-ins. I'd really like to keep it so that we don't need to perform an extra request when actually performing a check in, as discussed in #509. Is that relevant here?

@subzero10 @joshuap I think I followed your conversation about the project_id thing but I am not super sure if I can draw any conclusions regarding this PR from it right now? :)

I don't think there are any conclusion to draw just yet, we just need to hash this out before moving forward! 😄

Hey all, an update on this PR:

We're going to hold off on merging this for now, and I may close it at some point—but we may pick it back up next year. We just have other priorities to focus on for the moment.

In the meantime, I would like to finish the discussions we are having here so that we'll be in a good place to move forward if/when we come back to it. So let's continue discussing the details on the few unresolved threads above, but otherwise we can shift focus to other projects.

(@stympy and @subzero10 — please ignore the review requests above, I was just resetting the approvals.)

I think this is OK for syncing things, and is a nicer default experience. Since we already have the per-check-in project_id option in this PR, is there any harm in leaving it as an override if some people want to sync check-ins to other projects?

Since we now have the ability to map a project api key to a project id (https://github.com/honeybadger-io/honeybadger/pull/4368), I would vote to remove entirely project_id from config. If we have requests to support multiple projects, we can consider bringing it back in the future.

If we were to change something in the PR, it would be:

• remove project_id from config
• before synchronizing check-ins, first fetch the project id to be used for the synchronization
• use slug instead of name to synchronize check-ins
• modify validation to require slug to be present
• modify validation to make name optional (make sure to test this because it's not clear in the API doc whether it's optional)

I'm less certain about performing check-ins. I'd really like to keep it so that we don't need to perform an extra request when actually performing a check in, as discussed in #509. Is that relevant here?

For performing check-ins, we will use api key and slug to construct the check-in url, which can be done separately for #509.

I think this is OK for syncing things, and is a nicer default experience. Since we already have the per-check-in project_id option in this PR, is there any harm in leaving it as an override if some people want to sync check-ins to other projects?

Since we now have the ability to map a project api key to a project id (honeybadger-io/honeybadger#4368), I would vote to remove entirely project_id from config. If we have requests to support multiple projects, we can consider bringing it back in the future.

This works for me. 👍

If we were to change something in the PR, it would be:

• remove project_id from config
• before synchronizing check-ins, first fetch the project id to be used for the synchronization
• modify validation to require slug to be present

See my #509 (comment) for one more suggestion—can we use the slug instead of the name as the sync identifier when the check-in ID isn't available? This would reduce the check-in name to just a label again, which I think is better—people might want to rename them without affecting sync. (Let me know if I'm missing something here.)

I'm less certain about performing check-ins. I'd really like to keep it so that we don't need to perform an extra request when actually performing a check in, as discussed in #509. Is that relevant here?

For performing check-ins, we will use api key and slug to construct the check-in url, which can be done separately for #509.

Yep, I think this a nicer experience—you don't need to worry about project_id at all as the user. 👍

See my #509 (comment) for one more suggestion—can we use the slug instead of the name as the sync identifier when the check-in ID isn't available? This would reduce the check-in name to just a label again, which I think is better—people might want to rename them without affecting sync. (Let me know if I'm missing something here.)

No you are not missing anything, one of the reasons to make slug required is because we will use it to sync the check-ins as well. I just forgot to type it in the changes to be made! I will modify the PR description and add the complete list of remaining todo items.

Closing this since we don't have the bandwidth currently. We'll keep the branch around and create a new PR if/when we resume this feature. Thanks everyon!