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

apmpackage: remove version directory #5414

Merged
merged 7 commits into from
Jun 8, 2021

Conversation

axw
Copy link
Member

@axw axw commented Jun 4, 2021

Motivation/summary

Remove the version from the apm integration package directory. It is not necessary, and the version bumping makes retaining git history more difficult. See: https://github.com/elastic/apm-server/commits/master/apmpackage/apm/0.3.0

With these changes, bumping the version should just be a matter of updating apmpackage/apm/manifest.yml and adding a changelog entry.

How to test these changes

Non-functional change.

axw added 5 commits June 4, 2021 15:46
@apmmachine
Copy link
Contributor

apmmachine commented Jun 4, 2021

💚 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

Expand to view the summary

Build stats

  • Build Cause: Pull request #5414 updated

  • Start Time: 2021-06-04T12:17:52.346+0000

  • Duration: 37 min 21 sec

  • Commit: 06d14df

Test stats 🧪

Test Results
Failed 0
Passed 6142
Skipped 120
Total 6262

Trends 🧪

Image of Build Times

Image of Tests

@@ -84,7 +81,7 @@ def bump(v):
# copy over the package and replace version in manifest and pipeline names
shutil.copytree(src, dst)
subprocess.check_call('rm -rf {0}'.format(os.path.join(dst, 'README.template.md')), shell=True)
cmd = 'find {0} -not -name "*.png" -type f -print0 | xargs -0 sed -i "" "s/{1}/{2}/g"'.format(
cmd = 'find {0} -not -name "*.png" -type f -exec sed -i -e "s/{1}/{2}/g" {{}} \\;'.format(
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 didn't look too deeply into it, but the script wouldn't run for me as it was -- I guess some discrepancy between the version of find shipped with macOS.

Copy link
Contributor

Choose a reason for hiding this comment

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

changes look good - I ran make update with the changed code and everything still worked fine on a macOS

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for testing!

@axw axw added the v7.14.0 label Jun 5, 2021
@axw axw marked this pull request as ready for review June 6, 2021 05:28
@axw axw requested a review from a team June 6, 2021 05:28
@axw axw merged commit 1876181 into elastic:master Jun 8, 2021
@axw axw deleted the apmpackage-no-version-dir branch June 8, 2021 03:23
mergify bot pushed a commit that referenced this pull request Jun 8, 2021
* apmpackage: remove version directory

The version directory should be added when copying
to package-storage, but it is not needed in this repo.
The version is maintained in manifest.yml.

* apmpackage: update package version to 0.3.0

* Update apmpackage instructions and script

* Makefile: update *-package targets

* add docker entrypoint to copy package to am/n.n.n

* apmpackage/cmd/gen-package: no version in path

* make fmt

(cherry picked from commit 1876181)

# Conflicts:
#	apmpackage/cmd/gen-package/main.go
axw added a commit that referenced this pull request Jun 8, 2021
* apmpackage: remove version directory (#5414)

* apmpackage: remove version directory

The version directory should be added when copying
to package-storage, but it is not needed in this repo.
The version is maintained in manifest.yml.

* apmpackage: update package version to 0.3.0

* Update apmpackage instructions and script

* Makefile: update *-package targets

* add docker entrypoint to copy package to am/n.n.n

* apmpackage/cmd/gen-package: no version in path

* make fmt

(cherry picked from commit 1876181)

# Conflicts:
#	apmpackage/cmd/gen-package/main.go

* Fix merge conflict

Co-authored-by: Andrew Wilkins <[email protected]>
@jalvz
Copy link
Contributor

jalvz commented Jun 8, 2021

I guess apm-integration-testing's --package-registry-apm-path will not work right out of the box when pointing it to apm-server/apmpackage; one would need to copy the package elsewhere and stick in the version folder.
just an fyi

@axw
Copy link
Member Author

axw commented Jun 8, 2021

Ah, I didn't know about that one. Indeed, we'd need to do something like https://github.com/elastic/apm-server/blob/master/testing/docker/package-registry/entrypoint.sh

@axw
Copy link
Member Author

axw commented Jun 8, 2021

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.

5 participants