Skip to content

Conversation

@AlexanderWert
Copy link
Member

@ghost
Copy link

ghost commented Mar 30, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-03-31T13:27:51.107+0000

  • Duration: 3 min 12 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /test linters : Run the Python linters only.

  • /test full : Run the full matrix of tests.

  • /test benchmark : Run the APM Agent Python benchmarks tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

basepi
basepi previously requested changes Mar 30, 2022
Copy link
Contributor

@basepi basepi left a comment

Choose a reason for hiding this comment

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

I would really like to avoid adding binary files (the .png) to the repo. Does it make sense to move them to a central repo? Perhaps something like elastic/apm-artifacts, and then pull them into the docs from there?

134KB isn't a lot, but every binary file will be downloaded in every clone from now to the end of time, and if the image changes git can't diff them so now both versions will be included in the repo forever.

@AlexanderWert
Copy link
Member Author

I would really like to avoid adding binary files (the .png) to the repo. Does it make sense to move them to a central repo? Perhaps something like elastic/apm-artifacts, and then pull them into the docs from there?

If there's another way to include the image I'm up for it. @bmorelli25 do you have an idea what we could do instead of uploading the image in this repo and embedding it in the docs?

@bmorelli25
Copy link
Member

I would really like to avoid adding binary files (the .png) to the repo. Does it make sense to move them to a central repo? Perhaps something like elastic/apm-artifacts, and then pull them into the docs from there?

If there's another way to include the image I'm up for it. @bmorelli25 do you have an idea what we could do instead of uploading the image in this repo and embedding it in the docs?

We're already including a lot of content in these docs from elastic/aws-apm-lambda/docs. I don't see why we can't store the images there too. I'll add them to elastic/apm-aws-lambda#158, cherry-pick to 8.1, and rerun the build.

@basepi
Copy link
Contributor

basepi commented Mar 31, 2022

Honestly it's fine if we want to store them here. I just have worked on projects with very large clone times due to binary files and want to at least discuss alternatives before we merge them.

@basepi basepi dismissed their stale review March 31, 2022 16:03

no longer relevant

Copy link
Contributor

@basepi basepi left a comment

Choose a reason for hiding this comment

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

These docs are excellent! Love the tabbed sections for the various ways lambda can be configured!

@AlexanderWert AlexanderWert merged commit 3566dc0 into elastic:main Apr 1, 2022
basepi added a commit that referenced this pull request Apr 6, 2022
* Improved lambda getting started docs (#1511)

* Improve lambda getting started docs

* fixed labels in configure-lambda-widget.asciidoc

* match java prereq language

* remove img and include from another repo

* fixed heading level of step 3

Co-authored-by: bmorelli25 <[email protected]>

* ci: packer_cache never fail (#1516)

* Fix sanic tests (#1518)

Co-authored-by: Alexander Wert <[email protected]>
Co-authored-by: bmorelli25 <[email protected]>
Co-authored-by: Victor Martinez <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants