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.

Sign up for GitHub

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

Jump to bottom

export the DiagnosticHandler type #891

Merged
merged 8 commits into from
Feb 5, 2025
Merged

export the DiagnosticHandler type #891

merged 8 commits into from
Feb 5, 2025
+209 −36

Conversation

jamesross3
Copy link
Contributor

No description provided.

@changelog-app
Copy link

changelog-app bot commented Jan 17, 2025 •
edited by jamesross3
Loading

Generate changelog in changelog/@unreleased

What do the change types mean?
  • feature: A new feature of the service.
  • improvement: An incremental improvement in the functionality or operation of the service.
  • fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
  • break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
    and external consumers of the service's API (e.g. customer-written software or integrations).
  • deprecation: Advertises the intention to remove service functionality without any change to the
    operation of the service itself.
  • manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
    performing database surgery, ...) at the time of upgrade for it to succeed.
  • migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?
  • ❗The break and manual task changelog types will result in a major release!
  • 🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
  • ✨ All others will result in a minor version release.

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

export the DiagnosticHandler type

Check the box to generate changelog(s)

  • Generate changelog entry

@jamesross3 jamesross3 marked this pull request as ready for review January 30, 2025 00:23
nmiyake
nmiyake requested changes Feb 5, 2025
View reviewed changes
Copy link
Contributor

@nmiyake nmiyake left a comment

Choose a reason for hiding this comment

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

Minor feedback, but core of change looks good

witchcraft/wdebug/api.go Show resolved Hide resolved
witchcraft/internal/wdebug/handlers.go Outdated
)

// DiagnosticHandler is deprecated in favor of the exported/non-external wdebug.DiagnosticHandler.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// DiagnosticHandler is deprecated in favor of the exported/non-external wdebug.DiagnosticHandler.
// DiagnosticHandler is deprecated in favor of the exported wdebug.DiagnosticHandler.

What does "/non-external" mean here? It looks like that interface is exported/external.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for catching, fixed

witchcraft/wdebug/api.go Show resolved Hide resolved
James Ross added 3 commits February 5, 2025 14:55
Add DiagnosticType.Validate() method. Invoke this during wdebug route…
5ad7224 
… registration.

Add DiagnosticHandler interface documentation.

Fix docs typo.
add changelog
4b1610d
newline at end of CL
b76a04d
nmiyake
nmiyake requested changes Feb 5, 2025
View reviewed changes
Copy link
Contributor

@nmiyake nmiyake left a comment

Choose a reason for hiding this comment

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

Thanks! Feedback on comment and looks like you need to run ./godelw license

witchcraft/wdebug/api.go Outdated Show resolved Hide resolved
@jamesross3 jamesross3 merged commit efed930 into palantir:develop Feb 5, 2025
5 checks passed
@autorelease3
Copy link

autorelease3 bot commented Feb 5, 2025

Released v2.90.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfackler sfackler sfackler left review comments

@nmiyake nmiyake nmiyake approved these changes

Assignees
No one assigned
Labels
autorelease
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jamesross3 @sfackler @nmiyake