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

docs: Improve code comments on transactions #921

Merged

Conversation

philipphofmann
Copy link
Member

Improve code comments on methods for capturing transactions.

@codecov-io
Copy link

Codecov Report

Merging #921 (9bc8968) into feat/performance-monitoring (436c1b7) will decrease coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@                       Coverage Diff                       @@
##           feat/performance-monitoring     #921      +/-   ##
===============================================================
- Coverage                        94.16%   94.10%   -0.06%     
===============================================================
  Files                               80       80              
  Lines                             3582     3582              
===============================================================
- Hits                              3373     3371       -2     
- Misses                             209      211       +2     
Impacted Files Coverage Δ
Sources/Sentry/SentryInstallation.m 86.36% <0.00%> (-9.10%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 436c1b7...9bc8968. Read the comment docs.

@philipphofmann philipphofmann self-assigned this Jan 29, 2021
@@ -45,27 +45,29 @@ SENTRY_NO_INIT
/**
* Captures a transaction and sends it to Sentry.
*
* @param transaction The Transaction to send to Sentry.
* @param transaction The transaction to send to Sentry.
Copy link
Contributor

Choose a reason for hiding this comment

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

L: our official docs sentry-docs always PascalCase names that are meant as features, which is the case of a Transaction, represents a feature and not merely the wording transaction itself.
wondering why would we do differently.

eg https://docs.sentry.io/product/releases/health/#crash
you can see the wording Discover and Issues using Pascal case.

I'm not against and fine with the changes, just wanted to understand the reasoning

Copy link
Member Author

Choose a reason for hiding this comment

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

Here, I think we are not referring to the feature but instead to the class. So I think "transaction" is appropriate. @brustolin, the discussion here could be interesting for you as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood

Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com>
@philipphofmann philipphofmann merged commit 352f07e into feat/performance-monitoring Feb 2, 2021
@philipphofmann philipphofmann deleted the docs/improve-transaction-docs branch February 2, 2021 10:18
philipphofmann added a commit that referenced this pull request Feb 2, 2021
Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com>
bruno-garcia added a commit that referenced this pull request Mar 11, 2021
* feat: performance monitoring

* build: Setup CI for performance monitoring branch

* feat: Performance API: Transaction (#908)

Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
Co-authored-by: Clang Robot <clang-robot@sentry.io>
Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

* test: Add captureTransaction to sample apps (#922)

* docs: Improve code comments on transactions (#921)

Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com>

* feat: Add missing properties to SpanContext and Transaction (#919)

Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

* feat: Adding SentrySpan to performance monitoring. (#932)

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>
Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
Co-authored-by: Clang Robot <clang-robot@sentry.io>

* ref: Remove SentryTransaction from public API. (#950)

Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>

* feat: Add transaction sampling properties to SentryOptions (#961)

* Transaction sampling

* Update CHANGELOG.md

* Format code

* Apply suggestions from code review

Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>

* Testing and fixes

* Using NSNumber for sampler return

* Update CHANGELOG.md

Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com>

* Comment Update

* Using NSNumber for tracesSampleRate

Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com>

* feat: Transaction in Sample App (#971)

* Some UI Sample

* Format code

* Apply suggestions from code review

Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>

* removing button to download image

* Adding status and fixing operation value

* Fixed Tests

Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>

* merge fix

* feat: Add sampling rules to SentryHub startTransaction. (#977)

* Changing sampled from bool to enum, using TracesSampler

* removing name from span

* SamplingTests

* changelog and code format

* Lint Fix

* Fix tests

* Update Samples/iOS-Swift/iOS-Swift/TraceTestViewController.swift

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

* Random generator for sampling

* missing comment

* remove sampler from SentryHub+TestInit.h

* Apply suggestions from code review

Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>
Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>

* meta: Add missing breaking change to Changelog

* performance changelog

Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
Co-authored-by: Dhiogo Brustolin <dhiogorb@gmail.com>
Co-authored-by: Clang Robot <clang-robot@sentry.io>
Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com>
Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants