Revert guild endpoints merge - incorrect target branch by knowlen · Pull Request #26 · knowlen/esologs-python

knowlen · 2025-07-21T05:27:52Z

Summary

Reverting PR #25 which was accidentally merged into main instead of dev.

Issue

PR Implement guild data endpoints #25 (guild endpoints) was merged directly into main
Per our workflow, only release branches should merge into main
Feature branches should target dev

Changes in this PR

✅ Reverts commit fc7c55f from main
✅ Fixes all test references to removed get_guild_reports method
✅ Updates documentation to remove references to unimplemented guild methods
✅ Corrects API coverage metrics in README (76% instead of 83%)
✅ Adds GitHub Action to validate PR source branches
- Only branches with release prefix can target main
- Others will fail the check with clear error message
✅ Adds CONTRIBUTING.md with branch workflow documentation

Test Status

All tests now pass after removing references to get_guild_reports
Documentation is consistent with available methods
API coverage metrics accurately reflect current state

Next Steps

Merge this revert
Add "Validate Source Branch" as required status check in branch protection
Create PR from feature/guild-endpoints to dev
Consider setting dev as default branch for new PRs

* Refactor client.py to reduce file size by 95% (#24) * Reorganize generated code into _generated subdirectory * Fix remaining test imports for ValidationError and generated modules * Suppress websockets deprecation warnings from generated code * Refactor client.py using factory patterns and mixins * Fix test failures in refactored client * Fix remaining test failures and add UNSET export for backward compatibility * Update documentation to reflect client refactoring * Fix type annotations and pre-commit issues * Remove client_save.py backup file * Fix all mypy type errors for pre-commit compliance * Fix kwargs passthrough issue in report methods Remove kwargs passthrough to execute() to prevent HTTP client errors. Update convenience methods to only pass expected parameters. Update test to match new behavior where kwargs are not passed through. * Add dev branch to CI/CD workflow triggers * Trigger Claude Code Review [review] * Remove Claude review trigger file * Allow manual triggering of Claude Code Review workflow * Fix Claude review workflow to support reopened PRs [review] Add 'reopened' to PR event types and condition check * Address high-priority reviewer feedback - Add Protocol for type safety with model_validate method - Cache regex patterns for performance improvement - Improve error messages to show available parameters - Add comprehensive documentation for method registration - Fix type annotations to satisfy mypy * Update documentation for refactored architecture - Fix markdown formatting in architecture.md for proper rendering - Update test counts from 278 to 310 tests (105 unit tests) - Update project structure to reflect new modular architecture - Add method_factory.py and param_builders.py to unit test docs - Document new mixins directory structure * fix formatting * Bump version to 0.2.0a3 Update version across all project files: - pyproject.toml - esologs/__init__.py - README.md - CLAUDE.md - docs/index.md - docs/changelog.md (with release notes) - docs/development/architecture.md --------- Co-authored-by: knowlen <knowlen@users.noreply.github.com> * Implement guild data endpoints - Add 4 new guild methods: get_guilds(), get_guild(), get_guild_attendance(), get_guild_members() - Create GraphQL queries for guild search, lookup, attendance, and members - Add parameter builder for guild attendance with proper defaults - Implement flexible get_guild() method supporting ID or name/server lookup - Add comprehensive unit tests (12 new tests) - Add integration tests (10 new tests) - Update guild-data.md documentation with examples for all new methods - Update API coverage from ~83% to ~90% (37/41 methods) * Fix unit test mocking for httpx.Response objects * Fix UNSET import in test_character_rankings.py * Address reviewer comments: fix self type annotation and import location * Fix guild documentation examples and add comprehensive tests * Convert relative imports to absolute imports for better maintainability * Add retry logic and resilience features for integration tests and API clients * Replace pilcrow (¶) with hash (#) for anchor links in documentation * Fix pytest configuration for retry logic in integration and docs tests --------- Co-authored-by: knowlen <knowlen@users.noreply.github.com>

This reverts commit fc7c55f.

claude · 2025-07-21T05:28:03Z

Claude finished @knowlen's task —— View job

PR Review Analysis

Reviewing this revert PR for code quality, potential issues, and workflow practices.

Todo List:

Read repository guidelines (CLAUDE.md) - No CLAUDE.md found
Analyze the scope and completeness of the revert
Check for broken imports or references
Review workflow and branching strategy changes
Assess test coverage impact
Evaluate security and performance implications
Provide comprehensive feedback

Review Summary

❌ Critical Issues - PR Not Ready for Merge

1. Incomplete Revert - Test Failures Guaranteed

21+ test references to removed get_guild_reports() method will cause immediate failures
Test files affected: test_guild_data_examples.py, test_report_search.py, test_quickstart_examples.py, etc.
CI/CD pipeline will fail on merge

2. Documentation Inconsistency

Guild data documentation references methods that no longer exist
README still shows outdated API coverage metrics

✅ Positive Aspects

1. Workflow Strategy

✅ Excellent workflow discipline - reverting incorrect merge target
✅ Proper branching workflow - enforcing dev → main flow
✅ Clean git revert - no merge conflicts or partial changes

2. Version Management

✅ Proper version revert - 0.2.0a3 → 0.2.0a2
✅ Consistent across files - pyproject.toml and init.py aligned

3. Architecture

✅ Clean restoration - removes complex factory patterns
✅ Simplified client.py - back to monolithic but stable structure

📋 Required Actions Before Merge

Fix Test References (Critical)

# These test files need updates:
- tests/docs/test_guild_data_examples.py
- tests/integration/test_report_search.py  
- tests/unit/test_report_search.py
- tests/sanity/test_api_sanity.py
- tests/docs/test_quickstart_examples.py
- tests/docs/test_report_search_examples.py

Update Documentation
- Remove references to advanced guild methods
- Update API coverage metrics in README
Validate CI Pipeline
- All tests must pass before merge
- Consider adding branch protection rules

🔍 Detailed Analysis

Revert Scope: This cleanly reverts 99 files (7,668 deletions, 1,799 additions), removing:

Guild endpoints factory architecture
Advanced guild methods (attendance, members, search)
Mixin pattern implementation
Generated code reorganization

Security: ✅ No security concerns - removing functionality rather than adding

Performance: ✅ Improves performance by removing complex factory overhead

Branching Strategy: ✅ Excellent - enforces proper dev branch workflow

🎯 Recommendation

DO NOT MERGE until test failures are resolved. The revert itself is architecturally sound, but the incomplete removal of dependent test code will break the build.

Next Steps:

✅ Merge this revert to main (after fixing tests)
✅ Re-create guild endpoints PR targeting dev branch
✅ Add GitHub Action to prevent non-release branches from targeting main

knowlen · 2025-07-21T05:51:16Z

Closing this PR as we've decided to use git reset instead of revert to fix the main branch. Main has been successfully reset to c369935.

knowlen and others added 2 commits July 20, 2025 22:10

Revert "Implement guild data endpoints (#25)"

27a1bf0

This reverts commit fc7c55f.

knowlen added 3 commits July 20, 2025 22:28

Add GitHub Action to validate PR source branches for main

9349088

Add CONTRIBUTING.md with branch naming and workflow guidelines

71de969

Fix test failures and documentation inconsistencies after revert

4a19183

knowlen force-pushed the main branch from fc7c55f to c369935 Compare July 21, 2025 05:51

knowlen closed this Jul 21, 2025

knowlen deleted the revert-guild-endpoints-from-main branch July 21, 2025 06:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert guild endpoints merge - incorrect target branch#26

Revert guild endpoints merge - incorrect target branch#26
knowlen wants to merge 5 commits intomainfrom
revert-guild-endpoints-from-main

knowlen commented Jul 21, 2025 •

edited

Loading

Uh oh!

claude bot commented Jul 21, 2025 •

edited

Loading

Uh oh!

knowlen commented Jul 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

knowlen commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Issue

Changes in this PR

Test Status

Next Steps

Uh oh!

claude bot commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Analysis

Todo List:

Review Summary

❌ Critical Issues - PR Not Ready for Merge

✅ Positive Aspects

📋 Required Actions Before Merge

🔍 Detailed Analysis

🎯 Recommendation

Uh oh!

knowlen commented Jul 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

knowlen commented Jul 21, 2025 •

edited

Loading

claude bot commented Jul 21, 2025 •

edited

Loading