-
Notifications
You must be signed in to change notification settings - Fork 204
feat: implement server versioning approach with semantic versioning support #296
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
base: main
Are you sure you want to change the base?
Conversation
…upport This implements the versioning approach agreed upon in issue #158: ## Key Changes ### Documentation - Added comprehensive versioning guide (docs/versioning.md) - Updated FAQ with versioning guidance and best practices - Enhanced schema description with SHOULD recommendations ### Schema Updates - Added 255-character limit for version strings in schema.json - Enhanced version field description with semantic versioning guidance ### Implementation - New isSemanticVersion() function for proper semver detection - Enhanced compareSemanticVersions() with prerelease support - Implemented compareVersions() strategy: * Both semver: use semantic comparison * Neither semver: use timestamp comparison * Mixed: semver versions always take precedence - Updated publish logic to determine "latest" using new strategy - Added version length validation (255 char max) ### Versioning Strategy 1. Version MUST be string up to 255 characters 2. SHOULD use semantic versioning for predictable ordering 3. SHOULD align with package versions to reduce confusion 4. MAY use prerelease labels for registry-specific versions 5. Registry attempts semver parsing, falls back to timestamp ordering 6. Clients SHOULD follow same comparison logic Resolves #158 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: adam jones <domdomegg@users.noreply.github.com>
@claude I think the logic for ordering etc. shouldn't happen in database/memory.go - it should happen a layer higher at the service layer, not persistence layer. |
Claude finished @domdomegg's task —— View job Successfully moved version ordering logic from database layer to service layer! ✅ Tasks Completed:
SummarySuccessfully refactored the version ordering logic as requested: ✅ Created
✅ Updated
✅ Simplified
✅ All tests pass - No breaking changes to existing functionality The architecture now properly separates concerns:
|
- Move isSemanticVersion, compareSemanticVersions, and compareVersions functions from internal/database/memory.go to new internal/service/versioning.go - Update service layer to handle version comparison business logic - Simplify database layer to only handle persistence - All tests pass, maintaining existing functionality Co-authored-by: adam jones <domdomegg@users.noreply.github.com>
|
||
### For Non-Semantic Versions | ||
If version parsing as semantic version fails: | ||
- The registry will always mark the version as latest (overriding any previous version) |
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.
This feels reasonable in isolation. but actually means that version
's total ordering is inconsistent with is_latest
, for packages with a mix of versions.
Not sure what is best to do here. Maybe this is fine. Maybe we should only update is_latest if it's now the latest in the total ordering spec below, idk.
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.
I think this behavior is necessary because the publisher does not indicate whether they are actually using semantic versioning. For example, if we said "semantic versions always take precedence", and a publisher is using non-semantic version numbers that sometimes appear to be semantic, then they are going to have a bad time. 😆
Unfortunately, as you noted, this means there is no natural sort order. Instead, we have to maintain an insertion order based on this logic.
```json | ||
{ | ||
"version_detail": { | ||
"version": "1.2.3-gke.1" |
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.
"version": "1.2.3-gke.1" | |
"version": "1.2.3-1" |
// getVersionsByName retrieves all versions of a server by name | ||
func (s *registryServiceImpl) getVersionsByName(ctx context.Context, name string) ([]*model.ServerDetail, error) { | ||
// Use the database's List method to find all servers with the same name | ||
entries, _, err := s.db.List(ctx, map[string]any{"name": name}, "", 1000) // Large limit to get all versions |
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.
fixed limit may cause problems, although no limit may cause dos. think this is okay for now.
@@ -0,0 +1,125 @@ | |||
package service |
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.
should have unit tests
|
||
### For Non-Semantic Versions | ||
If version parsing as semantic version fails: | ||
- The registry will always mark the version as latest (overriding any previous version) |
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.
I think this behavior is necessary because the publisher does not indicate whether they are actually using semantic versioning. For example, if we said "semantic versions always take precedence", and a publisher is using non-semantic version numbers that sometimes appear to be semantic, then they are going to have a bad time. 😆
Unfortunately, as you noted, this means there is no natural sort order. Instead, we have to maintain an insertion order based on this logic.
When comparing versions where one is semantic and one is not: | ||
- The semantic version is always considered higher | ||
- This ensures semantic versions take precedence in ordering |
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.
I think this conflicts with the above section ("If version parsing as semantic version fails"), and I think it may potentially result in undesirable behavior (see my comment on the above section).
## Examples | ||
|
||
### Valid Semantic Versions | ||
```json |
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.
```json | |
```javascript |
``` | ||
|
||
### Non-Semantic Versions (Allowed) | ||
```json |
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.
```json | |
```javascript |
This implements the versioning approach agreed upon in issue #158:
Key Changes
Documentation
Schema Updates
Implementation
Versioning Strategy
Resolves #158
Generated with Claude Code