Skip to content

THRIFT-6108: implemented exception documentation generation#3666

Open
SLAVONchick wants to merge 1 commit into
apache:masterfrom
SLAVONchick:master
Open

THRIFT-6108: implemented exception documentation generation#3666
SLAVONchick wants to merge 1 commit into
apache:masterfrom
SLAVONchick:master

Conversation

@SLAVONchick

@SLAVONchick SLAVONchick commented Jul 23, 2026

Copy link
Copy Markdown

Client: cpp,haxe,java,netstd,py

In this PR I've implemented the documentation generation of the exceptions thrown in the service methods.
I skipped the clients where there is no documentation is generated at all, so it only touched cpp,haxe,java,netstd,py.

  • Did you create an Apache Jira ticket? (Request account here, not required for trivial changes) - Yes
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"? - Yes
  • Did you squash your changes to a single commit? (not required, but preferred) - Yes
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"? - Yes
  • If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources. - Does involve code.

@mergeable mergeable Bot added c++ Pull requests that update C++ code haxe java Pull requests that update Java code c# Pull requests that update C# code Pull requests that update .NET code python compiler labels Jul 23, 2026
t_field* e = *e_iter;
std::string namespace_str = e->get_type()->get_program()->get_namespace(lang);

if (static_cast<std::string>(lang).compare("cpp") == 0) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, really, don't. We have OOP these days, like in "t_oop_generator", so we can use virtual methods to do such stuff.

PS: Pun intended - SCNR 🤣

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree, my bad. Fixed.

@Jens-G

Jens-G commented Jul 23, 2026

Copy link
Copy Markdown
Member

I like the idea. Would be glad to see this move forward.

#ifndef T_OOP_GENERATOR_H
#define T_OOP_GENERATOR_H

#include <boost/algorithm/string/replace.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC we don't use boost in the compiler. Can this be replaced by sth else?

@SLAVONchick SLAVONchick Jul 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found identical implementations in several generators and left only one in base t_oop_generator.

@mergeable mergeable Bot added c_glib dart Pull requests that update Dart code d Pull requests that update D code delphi javascript Pull requests that update Javascript code kotlin lua Pull requests that update Lua code nodejs typescript ocaml perl php labels Jul 24, 2026
@mergeable mergeable Bot added ruby Pull requests that update Ruby code testsuite labels Jul 24, 2026
@SLAVONchick

Copy link
Copy Markdown
Author

I like the idea. Would be glad to see this move forward.

Hi! Thank you for the review! I'm really looking forward to finish this work.

@SLAVONchick
SLAVONchick requested a review from Jens-G July 24, 2026 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c_glib c++ Pull requests that update C++ code c# Pull requests that update C# code Pull requests that update .NET code compiler d Pull requests that update D code dart Pull requests that update Dart code delphi haxe java Pull requests that update Java code javascript Pull requests that update Javascript code kotlin lua Pull requests that update Lua code nodejs ocaml perl php python ruby Pull requests that update Ruby code testsuite typescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants