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

Add zpages for service (servicez, pipelinez, extensionz) #894

Merged
merged 1 commit into from Apr 30, 2020

Conversation

bogdandrutu
Copy link
Member

Screen Shot 2020-04-29 at 5 21 25 PM

Screen Shot 2020-04-29 at 5 21 31 PM

Screen Shot 2020-04-29 at 5 21 35 PM

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
// See the License for the specific language governing permissions and
// limitations under the License.

// Code generated by "esc -pkg internal -o resources.go templates/"; DO NOT EDIT.
Copy link
Member Author

Choose a reason for hiding this comment

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

I can avoid submitting this generated code. Add it as required step in make to re-generate this file if not present, and add it to .gitignore.

Copy link
Contributor

Choose a reason for hiding this comment

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

So right now does it depend on the dev to update this when the templates are updated?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, also I could not find a way to add the copyright so an extra step is required. I kind of like it to generate this every time, but that may be annoying for ide users if they don't use make

Copy link
Contributor

Choose a reason for hiding this comment

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

What about a test to ensure that the version matches all the template files (just compare FS(true) vs FS(false))? In case of failure, the test should tell how to re-generate the file (including make to add header).

Copy link
Member Author

Choose a reason for hiding this comment

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

Was not trivial calling twice to create a template from the same data returns different objects that the assert.Equal sees them as different, sorry I will merge as is, we can improve later.

Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

Nice! LGTM.

// See the License for the specific language governing permissions and
// limitations under the License.

// Code generated by "esc -pkg internal -o resources.go templates/"; DO NOT EDIT.
Copy link
Contributor

Choose a reason for hiding this comment

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

So right now does it depend on the dev to update this when the templates are updated?

)

var (
fs = FS(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: rename fs to indicate that is actually reading from embedded assets, alternatively add a comment to indicate what false means in this case.

Copy link
Member Author

Choose a reason for hiding this comment

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

You cannot call with "true" from production code, because the file will not be present in the final binary. So need to call from the embedded data. I think this does not require comment, only tests should use true.

@bogdandrutu bogdandrutu merged commit 9a870ae into open-telemetry:master Apr 30, 2020
@bogdandrutu bogdandrutu deleted the zpages branch April 30, 2020 22:13
@flands flands added this to the Beta 0.4 milestone Jun 4, 2020
wyTrivail pushed a commit to mxiamxia/opentelemetry-collector that referenced this pull request Jul 13, 2020
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
…y#894)

Bumps [boto3](https://github.com/boto/boto3) from 1.19.4 to 1.19.5.
- [Release notes](https://github.com/boto/boto3/releases)
- [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst)
- [Commits](boto/boto3@1.19.4...1.19.5)

---
updated-dependencies:
- dependency-name: boto3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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

3 participants