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 version.py template (v2) #151

Merged
merged 3 commits into from
May 6, 2021
Merged

Add version.py template (v2) #151

merged 3 commits into from
May 6, 2021

Conversation

drasmuss
Copy link
Member

@drasmuss drasmuss commented May 5, 2021

Motivation and context:

The content of version.py is repeated in all of our repos, so we should template it.

Interactions with other PRs:

This is based on #39 , but so much has changed since that PR that it was easier to just start fresh.

The main difference functionally from #39 is that we don't internally generate the version string to be used statically in other files (e.g. setup.py and docs/conf.py). The version in those files is still determined dynamically by importing version.py. There were two reasons for this.

  1. In this PR templating version.py isn't mandatory, so we can't count on the static version info being available. However, we could work around this by having conditional logic that uses a static version if version.py is templated otherwise falls back to the dynamic import.

  2. This PR also supports calendar versioning, which complicates the generation of a static version. The bones-check during CI would be updating the static version each time, which means that we'd have to manually update the static version in every repo each month or bones-check would fail. Again we could work around this by having conditional logic that uses a static version if you're using semantic versioning but falls back to the dynamic version if you're using calendar versioning.

So as mentioned we could work around both of these limitations with some conditional logic. But I just wasn't sure that adding that complexity is worth it, given that we're doing everything dynamically currently and it's working well enough.

How has this been tested?

Added a test (and it's used to template version.py in this repo).

How long should this take to review?

  • Average (neither quick nor lengthy)

Types of changes:

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have read the CONTRIBUTING.rst document.
  • I have updated the documentation accordingly.
  • I have included a changelog entry.
  • I have added tests to cover my changes.
  • I have run the test suite locally and all tests passed.

@drasmuss drasmuss force-pushed the version-template branch 2 times, most recently from c7aed94 to af50a24 Compare May 5, 2021 15:09
Copy link
Member

@tbekolay tbekolay left a comment

Choose a reason for hiding this comment

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

Made two tiny changes, other than that this LGTM! Will merge on CI passing.

nengo_bones/scripts/format_notebook.py Outdated Show resolved Hide resolved
nengo_bones/version.py Outdated Show resolved Hide resolved
@tbekolay tbekolay merged commit 15d3b4b into master May 6, 2021
@tbekolay tbekolay deleted the version-template branch May 6, 2021 19:05
@drasmuss drasmuss mentioned this pull request Jan 15, 2024
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants