chore: remove unused Maven wrapper files (mvnw, mvnw.cmd, .mvn/)#42
Conversation
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is mostly file deletion/relocation, but it introduces two high-risk compatibility changes: removing the Maven Wrapper (likely breaking reproducible builds/CI) and changing the demo script’s default API endpoint to http://localhost/... (likely breaking local runs). If the wrapper removal is intentional, the repo should be updated to clearly document/install a required Maven version and update any CI that depended on ./mvnw.
Summary of changes
What changed
Build / tooling cleanup
- Removed Maven Wrapper artifacts:
mvnw,mvnw.cmd, and.mvn/wrapper/maven-wrapper.properties.
Documentation re-org / pruning
- Renamed
DEPLOYMENT.md→DEPLOYMENT_GUIDE.md. - Moved
RULE_EXAMPLES.md→documents/RULE_EXAMPLES.md. - Moved
ai-docs/**→documents/agents/**(including.obsidianconfigs). - Deleted several large documentation files:
EVENTARA_TECHNICAL_REPORT.mdPHASE_4_COMPLETION_SUMMARY.mdRELEASE_v0.1.1.mdTHRESHOLD_RULES_COMPLETE.md
Test assets removal
- Deleted Python-based test suite and its deps:
tests/test_threshold_rules.py,tests/requirements.txt.
Demo script tweak
- Updated default
API_URLintests/yc-demo-events.shfromhttp://localhost:8080/api/v1/eventstohttp://localhost/api/v1/events.
| #!/bin/sh | ||
| # ---------------------------------------------------------------------------- | ||
| # Licensed to the Apache Software Foundation (ASF) under one | ||
| # or more contributor license agreements. See the NOTICE file | ||
| # distributed with this work for additional information | ||
| # regarding copyright ownership. The ASF licenses this file | ||
| # to you under the Apache License, Version 2.0 (the | ||
| # "License"); you may not use this file except in compliance | ||
| # with the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, | ||
| # software distributed under the License is distributed on an | ||
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
| # ---------------------------------------------------------------------------- | ||
|
|
There was a problem hiding this comment.
Removing the Maven Wrapper is likely to break builds for contributors and CI environments that rely on ./mvnw (especially since the repo appears to be a Spring Boot Java project per the documentation). If you truly want to drop the wrapper, you should also update the build/run docs (and any CI configs) to explicitly require a locally installed Maven version and document the expected Maven version.
As-is, this PR removes the wrapper but does not show any compensating changes (e.g., README/DEPLOYMENT updates or CI pipeline updates), which is a high risk for reproducibility.
Suggestion
If the intent is to remove the wrapper, ensure all entry points are updated:
- Update docs to use
mvninstead of./mvnwand state a pinned Maven version (e.g.,3.9.x). - Update CI (GitHub Actions / other) to install that Maven version.
- Alternatively, keep the Maven Wrapper and only remove truly unused wrapper files.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit updating the docs/CI references accordingly (based on what exists in this repo).
| # Rule 3: Checkout Conversion Drop - checkout.success/checkout.started < 0.7 (min 5 events) | ||
| # Rule 4: Error Rate Anomaly - ERROR_RATE_CHANGE > 100% | ||
|
|
||
| API_URL="${API_URL:-http://localhost:8080/api/v1/events}" | ||
| API_URL="${API_URL:-http://localhost/api/v1/events}" |
There was a problem hiding this comment.
The default API_URL was changed from http://localhost:8080/api/v1/events to http://localhost/api/v1/events. This is almost certainly wrong for local development unless you have a reverse proxy on port 80 routing to the backend. The script header still reads like a local demo for Eventara, and most setups expose Spring Boot on 8080.
This change will cause the demo script to fail for the common case.
Suggestion
Revert the default to the typical backend port, or make it explicit:
API_URL="${API_URL:-http://localhost:8080/api/v1/events}"If you want to support both, consider splitting into API_HOST/API_PORT vars or detecting when localhost is used.
Reply with "@CharlieHelps yes please" if you want me to add a commit restoring the safer default and optionally improving configurability.
1 participant
Remove other useless files