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

Feature/msteams #1914

Merged
merged 12 commits into from
May 21, 2021
Merged

Feature/msteams #1914

merged 12 commits into from
May 21, 2021

Conversation

vincentbriglia
Copy link
Contributor

@vincentbriglia vincentbriglia commented Mar 25, 2021

What Changed

image

to

image

Why

Todo:

  • Add tests
  • Add docs
  • Fix the URL's generated for independently versioned packages

Change Type

Indicate the type of change your pull request is:

  • documentation
  • patch
  • minor
  • major

Note: I added you @hipstersmoothie to a teams channel as a guest, so you can try this out. I have also forwarded you the webhook URL

🐤 Download canary assets:

auto-linux--canary.1914.24254.gz
auto-macos--canary.1914.24254.gz
auto-win.exe--canary.1914.24254.gz

📦 Published PR as canary version: under canary scope @[email protected]

✨ Test out this PR locally via:

npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
# or 
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]

icrosoft Teams accepts markdown, as long as the content was escaped/serialized to a JSON string first
Mentions don't actually work when posted through the teams/office api
instead of baking in the URL at the top
@adierkens adierkens added the minor Increment the minor version when merged label Mar 25, 2021
@vincentbriglia
Copy link
Contributor Author

@hipstersmoothie question about the format for the response, to write tests for a release with multiple versions and packages, what is the response?

would the mockresponse look something like this?:

const mockResponse = [
  {
    data: {
      html_url: "https://git.hub/some/project/releases/v1.0.0",
      name: "v1.0.0",
    },
  },
  {
    data: {
      html_url: "https://git.hub/some/project/releases/v2.0.0",
      name: "v2.0.0",
    },
  },
];

@hipstersmoothie
Copy link
Collaborator

Probably more like:

const mockResponse = [
  {
    data: {
      html_url: "https://git.hub/some/project/releases/[email protected]",
      name: "[email protected]",
    },
  },
  {
    data: {
      html_url: "https://git.hub/some/project/releases/[email protected]",
      name: "[email protected]",
    },
  },
];

MS Teams might add this functionality later
@vincentbriglia
Copy link
Contributor Author

right, so - teams doens't support the atTarget functionality and based on the tests and some of the responses i'm getting, i'm getting conflicting results for the markdown returned. I will try this out with the canary version

@codecov
Copy link

codecov bot commented Mar 26, 2021

Codecov Report

Merging #1914 (5fa091a) into main (5d33773) will decrease coverage by 0.00%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1914      +/-   ##
==========================================
- Coverage   80.17%   80.16%   -0.01%     
==========================================
  Files          66       66              
  Lines        5386     5375      -11     
  Branches     1252     1248       -4     
==========================================
- Hits         4318     4309       -9     
+ Misses        708      707       -1     
+ Partials      360      359       -1     
Impacted Files Coverage Δ
packages/core/src/log-parse.ts 100.00% <ø> (ø)
packages/core/src/git.ts 87.53% <33.33%> (-0.25%) ⬇️
plugins/microsoft-teams/src/index.ts 85.96% <100.00%> (+1.90%) ⬆️

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 bd6b041...5fa091a. Read the comment docs.

@vincentbriglia
Copy link
Contributor Author

The url's generated f.ex
https://github.com/LEGO/klik/releases/tag/%40lego/klik-ui-popover%400.5.3|@lego/[email protected]
don't work,

comparing, ofcourse does
https://github.com/LEGO/klik/compare/@lego/[email protected]...@lego/[email protected]

but i don't really know how to get that created here.

@hipstersmoothie
Copy link
Collaborator

It looks like the code for creating a list of slack formated links is still in there. I think you'd need to create an addon for each release

@vincentbriglia
Copy link
Contributor Author

just not sure how to get the "previous release" in the context of multiple independently versioned packages. I know it comes from auto, newVersion in the afterRelease hook, I will need some help here.

@reintroducing
Copy link

@vincentbriglia this is fantastic, thank you!

@hipstersmoothie @adierkens what needs to be done to get this merged? I know @vincentbriglia said something about independently versioned packages, but it looks like he has made the applicable formatting changes for what was already in the Teams plugin thus far, and this is a huge improvement over whats currently being spit out by the Teams plugin, so can we at least get this merged to fix that portion?

@hipstersmoothie
Copy link
Collaborator

Fix the merge conflict and I'll merge ASAP!

@reintroducing
Copy link

@vincentbriglia Mind fixing the merge conflict and updating the PR?

@vincentbriglia
Copy link
Contributor Author

@hipstersmoothie please check the last couple commits, I've fixed it - had some Tapable issues - I am using Yarn, fixed it by installing the external type declarations for Tapable

@reintroducing
Copy link

@hipstersmoothie Any chance you can take a look at this?

@hipstersmoothie
Copy link
Collaborator

Sorry I started a new job and have been super busy! Not to mention I also started a podcast 😅 I will for sure look at this this weekend. But if someone can find a way to make the tests pass before then I'll review and merge.

@reintroducing
Copy link

@hipstersmoothie understood, so you're no longer at Intuit? and actively working on auto?

all that to say, any updates on this? :)

@hipstersmoothie
Copy link
Collaborator

Yeah no longer at intuit but still an avid user of auto thought! Working at a startup constrains my time in much different ways though. @vincentbriglia I've fixed the issue, can you give me push access to your fork?

@vincentbriglia
Copy link
Contributor Author

@hipstersmoothie done

@hipstersmoothie hipstersmoothie merged commit 1049d7c into intuit:main May 21, 2021
@reintroducing
Copy link

@vincentbriglia @hipstersmoothie thanks guys, much appreciated!

@hipstersmoothie if you recall our convo about a screenshot of the Slack output added to the documentation (which you've already done), maybe you can use the above image to do the same for the Teams docs page to show how the output will look?

@reintroducing
Copy link

@vincentbriglia @hipstersmoothie hmm, i just ran this and this was the output:
Screen Shot 2021-05-23 at 7 56 58 AM

@hipstersmoothie hipstersmoothie deleted the feature/msteams branch May 26, 2021 00:30
@hipstersmoothie
Copy link
Collaborator

Once we fix the output we can do that ;P Any idea on the fix for that?

@reintroducing
Copy link

Not sure what happened with the output. @vincentbriglia have you tried this yet? How did you get the original output in your screenshot?

@vincentbriglia
Copy link
Contributor Author

i haven't tried it, i will try it out tomorrow - am on a different product atm :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants