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

Fix #24 -- Move config to pyproject.toml #25

Merged
merged 4 commits into from
Nov 28, 2023

Conversation

codingjoe
Copy link
Contributor

@codingjoe codingjoe commented Nov 24, 2023

The python community has widely adopeted PIP 621. Therefore, it seems the best place to implement it in this package too.

There are two key changes:

  1. The config moves to tool.importmap.
  2. The importmap config allows adding multiple entries.

Additionally, this commits removes two dependencies:

  • marshmellow
  • tomli (python_version>=12.0)

Fix #24

@codingjoe codingjoe marked this pull request as ready for review November 24, 2023 16:50
@codingjoe
Copy link
Contributor Author

codingjoe commented Nov 24, 2023

@davegaeddert I have it a try, hope you like it 👍 I tried to keep it as simple as possible.
The README file, is a probably good place to start reviewing this patch.

The python community has widely adopeted PIP 621. Therefore,
it seems the best place to implement it in this package too.

There are two key changes:

1. The config moves to `tool.importmap`.
2. The importmap config allows adding multiple entries.

Additionally, this commits removes two dependencies:
* marshmellow
* tomli (python_version>=12.0)
Copy link
Member

@davegaeddert davegaeddert left a comment

Choose a reason for hiding this comment

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

Thanks @codingjoe! I'm good with going this direction, and I like the way you did it.

@davegaeddert davegaeddert merged commit df44b1b into dropseed:master Nov 28, 2023
5 checks passed
@github-actions github-actions bot mentioned this pull request Feb 1, 2023
3 tasks
@codingjoe codingjoe deleted the issues/24 branch November 28, 2023 20:33
@davegaeddert
Copy link
Member

Just released this as version 0.3.0.

@codingjoe
Copy link
Contributor Author

Woop, woop 🙌 Thanks! I am very keen on pushing this forward. I believe the concept is a big step forward for Django projects. If you don't mind, I would spend the weekend on mapping out a couple more suggestions to further push this package towards wider adoption. I'd probably start with adding some more documentation and technical writing. Is that cool with you?

@davegaeddert
Copy link
Member

Yeah I'm happy to look at whatever! No promises on getting it in obviously, but it seems like we're aligned for the most part. Let me know what you're thinking and I'll try to be responsive.

(FWIW I'd prefer docs to be in markdown vs rst — I have no interest in sphinx.)

Honestly the biggest thing I'm personally interested in is "vendoring" (downloading) the dependencies instead of just linking to the CDNs. I know importmap-rails has that as an option, and I think talked about making that the default behavior (or maybe they even changed it?). Anyway, the more I've used it, the more I've wanted it to just work that way. Bigger question/project, but throwing it out there if you're going to be thinking about things.

@codingjoe
Copy link
Contributor Author

Yeah I'm happy to look at whatever! No promises on getting it in obviously, but it seems like we're aligned for the most part. Let me know what you're thinking and I'll try to be responsive.

No worries, I also reject 2/3 of all proposals on my projects. Good maintenance requires vision 👍

(FWIW I'd prefer docs to be in markdown vs rst — I have no interest in sphinx.)

Same here 😉

Honestly the biggest thing I'm personally interested in is "vendoring" (downloading) the dependencies instead of just linking to the CDNs. I know importmap-rails has that as an option, and I think talked about making that the default behavior (or maybe they even changed it?). Anyway, the more I've used it, the more I've wanted it to just work that way. Bigger question/project, but throwing it out there if you're going to be thinking about things.

Yes, vendoring, local/offline development, dependabot updates. I will collect my thoughts in separate issues 🤓

@davegaeddert davegaeddert changed the title Fix #24 -- Move config to pyproject.yaml Fix #24 -- Move config to pyproject.toml Nov 29, 2023
@codingjoe
Copy link
Contributor Author

Hello again 👋,

I took some time to rethink the entire concept and ended up implementing an entirely different approach from scratch that addresses my previous concerns.

I chose to do this as part of a new project because entirely different implementation while achieving a similar goal. I just published it here: https://github.com/codingjoe/django-esm

If you are interested in merging those projects and combining efforts, please let me know.

Cheers,
Joe

@davegaeddert
Copy link
Member

Thanks @codingjoe! To be honest, I don't have time at the moment to think this all the way through, but I'll definitely take a closer look at your approach when I come back to this.

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.

alter PackageSchema
2 participants