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

Proposal: remove discussions code #9649

Closed
4 of 6 tasks
keegancsmith opened this issue Apr 8, 2020 · 18 comments · Fixed by #10659
Closed
4 of 6 tasks

Proposal: remove discussions code #9649

keegancsmith opened this issue Apr 8, 2020 · 18 comments · Fixed by #10659
Assignees
Labels
debt Technical debt. proposal

Comments

@keegancsmith
Copy link
Member

keegancsmith commented Apr 8, 2020

Discussions is an unused product and has remained that way for a long time. I propose we remove the code from our codebase. I'm not sure of a reason to keep it, but I may be missing some context.

cc @sqs @christinaforney @slimsag

@keegancsmith keegancsmith added debt Technical debt. proposal labels Apr 8, 2020
@slimsag
Copy link
Member

slimsag commented Apr 8, 2020

For historical context, the main reason for not removing it was that we intended to revive it and I could either maintain a separate branch with it or maintain it in master -- in either case, we did not remove it because it was not an impedance to anyone's work and we had agreed if it caused anyone effort, etc. that we would just move it to a separate branch.

At this point, I agree it seems unlikely we are going to be bringing it back soon and if we did I would want to make a number of important changes to it, so I am fine removing it for now.

@keegancsmith
Copy link
Member Author

Yeah I agree it is better to just remove it. Even though it seems to not be causing any direct issues, it is a source of potential confusion in our codebase. I think once we have a product thumbs up from @christinaforney we can go ahead and do that.

@felixfbecker
Copy link
Contributor

felixfbecker commented Apr 9, 2020

I would like to offer a data point that the discussion code has taken unproportionally much effort to maintain in the web codebase compared to other parts of the web codebase (not surprising), when upgrading dependencies, adapting new TypeScript versions, stricter lint settings etc. I'm often faced with the decision: Do I put in extra work to refactor and make everything work in this unused part of the codebase, or just work around the particular issue at hand in this part of the codebase, which makes the problem even worse over time.

@keegancsmith
Copy link
Member Author

That sounds like exactly the motivation that would trigger us removing discussions.

@christinaforney
Copy link
Contributor

We've recently had a prospect find this functionality and suggest some features. This is the first (only?) time I've seen this happen, and don't think it's being used in production yet. I don't think this is reason to keep the functionality, and if it's caused issues for the web team it sounds worth removing. This helps us focus on other parts of the product that we believe will have a greater impact.

@keegancsmith
Copy link
Member Author

I've deprecated the graphQL apis, so we can go ahead and remove it in 3.16.

We've recently had a prospect find this functionality and suggest some features. This is the first (only?) time I've seen this happen, and don't think it's being used in production yet. I don't think this is reason to keep the functionality, and if it's caused issues for the web team it sounds worth removing. This helps us focus on other parts of the product that we believe will have a greater impact.

I think a lot of the code can be revived if we do want to focus on this again. I'd prefer that approach than keeping around all the code now. Looking at our roadmap I assume we won't have much time to work on discussions soon.

@felixfbecker
Copy link
Contributor

I agree with @keegancsmith - if we want to have it back, at least for the web part, it would probably take a few hours to merge the old code back in from commit history.

if it's caused issues for the web team it sounds worth removing

Just to be clear, I wouldn't claim it "caused issues", but it has a non-zero maintenance cost. I think that cost has exceeded the cost it would take to bring it back.

@christinaforney
Copy link
Contributor

Great - then go for it! As you both said, we can always revive this code if we need to :)

@keegancsmith
Copy link
Member Author

I just removed sourcegraph/code-discussions from https://sourcegraph.com/organizations/sourcegraph/settings. Lets see if traffic goes to zero on the discussions endpoint. There is the possibility there are other test orgs/uses who have it explicitly enabled. @felixfbecker If we merge in #10262 I believe that will also effectively switch it off for everyone?

@felixfbecker
Copy link
Contributor

Yes that would switch it off, unless someone uses the API ;)
But again - the clearest way would be to disable the feature flag completely in site config, which also disables backend code:

image

@mastercoms
Copy link

mastercoms commented May 8, 2020

We use the discussions option on our own Sourcegraph instance to help us navigate our large (sometimes third-party) code bases effectively, especially for new hires. What can we do to make sure that we keep this functionality, short of not updating to 3.16? Anyone know of any alternatives?

@sqs
Copy link
Member

sqs commented May 8, 2020

@mastercoms Glad to hear you were finding code discussions useful. It was and still is highly experimental, and we weren't aware of anyone using it on an ongoing basis (until you mentioned this just now). Are you able to share any more information, such as the following?

  • What you like and don't like about it
  • What other alternatives you've tried or looked at
  • How many people at your company use it
  • How many comments total (and average per day)

I hope you'll understand our intent to remove this feature given the experimental nature and extremely low usage. We are generally excited by the idea of code discussions and will revisit it if/when it's a priority (and hearing the feedback above helps us prioritize it).

In the meantime, would https://www.codestream.com/ solve your problem?

You can extract all your code discussions data using the Sourcegraph GraphQL API for your records.

(Finally, if you are an existing customer, or if having continued access to this feature is really important to your company and you would pay, please reach out to us at support@sourcegraph.com. I'm not able to tell from your GitHub account if you're an existing customer.)

@mastercoms
Copy link

Hi, thank you for the swift and comprehensive reply.

I like the ability to see a list of all discussions on a repo, which allows us to navigate code quickly. If there was some sort of bookmarking feature, that would suffice since we don't see much value from actually having a threaded conversation with markdown. We largely just use the automatic header feature. One thing I don't like is no ability to edit discussions comments.

We haven't used any alternatives, as this workflow was introduced to us when we started using Sourcegraph, and we have grown to love it.

We have about 15 people using this feature at our company. We have about 100 discussions at the moment. About 1 to 5 comments are made per day, all of them being new threads, unless we need to correct a comment, due to the lack of an edit feature.

CodeStream definitely looks like something we can use, thank you.

We are not paying customers, and my intention was not to prevent this change from taking place. It seems very reasonable. I just wanted to see what actions we should take on our end, and I think CodeStream is a viable solution to that.

Let me know if you have any other questions. I really appreciate your software, and would like to have a small part in helping it improve :).

@sqs
Copy link
Member

sqs commented May 8, 2020

Thank you! 😄 This is very helpful, and you are definitely playing a part in helping us improve. If you have any problems exporting your data via the Sourcegraph GraphQL API, feel free to post an issue here.

One other alternative is to create a simple Sourcegraph extension (https://docs.sourcegraph.com/extensions) for bookmarking. It's essentially similar to writing an editor (eg VS Code) extension, except it runs on Sourcegraph and executes client-side in your browser. If you are interested in doing that, that would continue to work on all future versions of Sourcegraph--and it would also work on your code host (GitHub/GitLab/Bitbucket Server/Phabricator) with our code host integrations. I'm happy to screenshare with you to help if you're interested and run into any questions that aren't easily answered over text (just email me sqs@sourcegraph.com).

@uwedeportivo
Copy link
Contributor

Dear all,

This is your release captain speaking. 🚂🚂🚂

Branch cut for the 3.16 release is scheduled for tomorrow.

Is this issue / PR going to make it in time? Please change the milestone accordingly.
When in doubt, reach out!

Thank you

@felixfbecker
Copy link
Contributor

I noticed there are still discussions route handlers:

router.Get(routeThreads).Handler(handler(serveBrandedPageString("Threads")))

@felixfbecker felixfbecker reopened this Jun 11, 2020
@keegancsmith
Copy link
Member Author

keegancsmith commented Jun 11, 2020 via email

@slimsag slimsag removed this from the 3.16 milestone Jul 1, 2020
@slimsag
Copy link
Member

slimsag commented Aug 24, 2020

I assume this is done or we aren't planning to work on it more.

@slimsag slimsag closed this as completed Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Technical debt. proposal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants