Skip to content

Added dpp spatial extent#253

Open
minhajuddin2510 wants to merge 6 commits into
mainfrom
added_dpp_spatial_extent
Open

Added dpp spatial extent#253
minhajuddin2510 wants to merge 6 commits into
mainfrom
added_dpp_spatial_extent

Conversation

@minhajuddin2510
Copy link
Copy Markdown
Collaborator

No description provided.

@jqnatividad
Copy link
Copy Markdown
Collaborator

Thanks @minhajuddin2510 for this work. Before this can land, a few concerns I'd like to flag for a follow-up version:

Scope. The PR title says "Added dpp spatial extent" but it also touches ai_suggestions.py, AI-suggestion JS / templates / web assets, jinja2 helpers, and plugin.py — looks like multiple unrelated features bundled together (~1900 lines across 12 files). Could you split this into smaller focused PRs? A reviewer can give a much better review on one feature at a time, and a regression in one feature won't block the others.

Description. The PR body is empty. For changes this size, a description covering (a) what each feature does, (b) why it's needed, (c) any operator-facing config / migration steps, (d) how it was tested would unblock review substantially.

Rebase needed. The PR targets the pre-migration codebase — it modifies ckanext/datapusher_plus/jobs.py, which the Prefect 3 migration (#277) deleted in favour of ckanext/datapusher_plus/jobs/stages/*.py. The current call sites are different too.

I'd suggest closing this and reopening one PR per feature (spatial extent / AI suggestions / etc.) on top of current main with descriptions. Happy to review each as it comes in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants