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

feat: Add upm target #209

Merged
merged 18 commits into from
May 7, 2021
Merged

feat: Add upm target #209

merged 18 commits into from
May 7, 2021

Conversation

bitsandfoxes
Copy link
Contributor

@bitsandfoxes bitsandfoxes commented Apr 29, 2021

Unity SDK works in two repositories:

https://github.com/getsentry/sentry-unity <- SDK
https://github.com/getsentry/unity <- release repo for the user
Unity users add our package by providing a git URL to their package manager. Versioning and potential rollbacks for them are made available via tags (i.e. https://github.com/getsentry/unity.git#x.x.x).

Release steps:

  1. Get the prepared artifact (a single .zip containing the package)
  2. Pull the release repository
  3. Replace everything in that repo with the contents of the .zip
  4. Update tag & push

@bitsandfoxes bitsandfoxes requested review from BYK, HazAT and tonyo April 29, 2021 09:49
@tonyo
Copy link
Contributor

tonyo commented Apr 29, 2021

I see this is a Draft, but just a few things not to forget about:

  • Let's update the README with the docs about the new target
  • Let's make sure that tests and linters are green. @BYK what's the linting story right now? Do we use eslint? prettier? Also, I'm not sure we're running them as part of CI now.

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

Looking good, maybe drop the draft tag?

README.md Outdated Show resolved Hide resolved
src/targets/upm.ts Outdated Show resolved Hide resolved
src/targets/upm.ts Outdated Show resolved Hide resolved
src/targets/upm.ts Outdated Show resolved Hide resolved
@BYK
Copy link
Member

BYK commented May 3, 2021

Added the lint automation and fixes here: #214

Copy link
Member

@BYK BYK left a comment

Choose a reason for hiding this comment

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

I think these relative imports will fix the test issues. I'll look into why this is breaking separately.

Btw is it possible to have some tests for this target?

src/targets/upm.ts Outdated Show resolved Hide resolved
src/targets/upm.ts Outdated Show resolved Hide resolved
@BYK BYK marked this pull request as ready for review May 7, 2021 12:09
@BYK BYK requested a review from bruno-garcia May 7, 2021 12:09
Copy link
Member

@lobsterkatie lobsterkatie left a comment

Choose a reason for hiding this comment

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

Mostly just proofreading. (You can take the girl out of copyediting...) Otherwise LGTM!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/targets/__tests__/upm.test.ts Show resolved Hide resolved
src/targets/__tests__/upm.test.ts Outdated Show resolved Hide resolved
src/targets/__tests__/upm.test.ts Outdated Show resolved Hide resolved
src/targets/github.ts Show resolved Hide resolved
src/targets/github.ts Show resolved Hide resolved
src/targets/upm.ts Outdated Show resolved Hide resolved
src/targets/upm.ts Outdated Show resolved Hide resolved
src/targets/__tests__/upm.test.ts Outdated Show resolved Hide resolved
BYK and others added 3 commits May 7, 2021 17:58
@BYK BYK enabled auto-merge (squash) May 7, 2021 19:37
@BYK BYK removed request for HazAT and tonyo May 7, 2021 19:37
@BYK BYK merged commit 30f5cc0 into master May 7, 2021
@BYK BYK deleted the feat/upm branch May 7, 2021 19:38
@bruno-garcia
Copy link
Member

Thanks a lot @BYK , @bitsandfoxes , @tonyo , @lobsterkatie !!!

🥳 🥳

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.

6 participants