-
Notifications
You must be signed in to change notification settings - Fork 367
Airship Actions Destination #4946
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
Conversation
6. Click **Create Destination**. | ||
7. Enter your Access Token and App Key. You can get your access token and app key by going to your Airship project and navigating to **Settings > Partner Integrations** and selecting Segment. Following the instructions there will create a Tag Group, Attributes, and provide the Access Token and App Key. | ||
8. Select the appropriate data center. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jumpingGrendel this is the comment that @joe-ayoub-segment left from the previous PR (#4921). I'm copying it over to this PR so that you can address them here:
We could add some lines to explain which Preset Mappings are available, etc.
I think we should add a new heading, and explain how identifiers work. It's important for the customer to know that Segment won't create a user profile in Airship. i.e the "Airship Named User ID" field is required, and the value needs to be for a pre-existing Named User in Airship. Segment events cannot be used to create a user profile in Airship... etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stayseesong and @joe-ayoub-segment have a look I added some deeper explanations for some of the behavior/concepts. thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jumpingGrendel where did you add the edits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm @stayseesong when you closed the PR and reopened it did it lose the connection with my fork? urbanairship@8a4e417
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jumpingGrendel I didn't reopen the PR that you created. I had to close that PR out because the docs team was unable to edit it, so I had to create a new PR where the team can edit it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stayseesong sorry I was unclear! The old PR wanted to merge my fork/branch into segmentio develop whereas for this new PR you've created a new branch in segmentio. I tried making my changes to your branch but I don't have permission.
john @ cantankerous ~/projects/segment-docs (PINT-1842)
└─ $ ▶ git push origin PINT-1842
Enumerating objects: 15, done.
Counting objects: 100% (15/15), done.
Delta compression using up to 8 threads
Compressing objects: 100% (7/7), done.
Writing objects: 100% (8/8), 648 bytes | 648.00 KiB/s, done.
Total 8 (delta 6), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (6/6), completed with 6 local objects.
remote: Bypassed rule violations for refs/heads/PINT-1842:
remote:
remote: - Changes must be made through a pull request.
remote:
To github.com:urbanairship/segment-docs.git
8a4e417..095c294 PINT-1842 -> PINT-1842
john @ cantankerous ~/projects/segment-docs (PINT-1842)
└─ $ ▶
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jumpingGrendel that's strange, not sure why you're unable to edit the PR... Are the changes in here what you want to add to the docs? Just making sure, you don't want to include the Named User ID section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, sorry, I corrected it here.
I can't edit your new branch because it's in the segmentio repo. As I understand it, I develop on a remote fork and create a PR from there, that way I can continue to edit/update as needed until you (the repo admin) decide to merge it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @jumpingGrendel. Apologies for the delays and disconnect here. We've run into similar issues before wherein our GitHub org does not work nicely with the GitHub org of our partners.
I will manually add your updates from the commit here
✅ Deploy Preview for segment-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@jumpingGrendel I've updated the contents of the article, and incorporated available metadata (settings, actions, presets) from the API. Please have a look at the build preview here. Please @ mention me directly if there is anything missing. If things look good, we'll deploy it. |
Thanks @markzegarelli ! Only thing missing is my latest change to my remote fork: ad905be |
Hi @jumpingGrendel, I've included the content about Named Users here. The content you for Additional Information is largely captured in the content for each action. For example, Custom Events. I will re-add your Additional Information section, but best practice would be to include the information here in the descriptions for the relevant fields in the code, for example here for the Custom Event Name field. |
Thanks @markzegarelli it looks great, I am fine with it being merged if you are. |
Great, we'll get this merged in our deploy tmrw! |
Thank you for your contribution! Your pull request is merged, but may take a day or two to appear on the site. |
Proposed changes
Merge timing
Once ready to go and stakeholders addressed the comments
Related issues (optional)