Skip to content

Feature:support alter relativedelta field on admin#7

Open
ChandlerBent wants to merge 2 commits intoCodeYellowBV:masterfrom
ChandlerBent:master
Open

Feature:support alter relativedelta field on admin#7
ChandlerBent wants to merge 2 commits intoCodeYellowBV:masterfrom
ChandlerBent:master

Conversation

@ChandlerBent
Copy link

Hi

I am looking for the relativedelta field and want to use on admin.
Then found it that solve what I want.
Bug it doesn't support alter on admin.
So I do some modify to support admin.

@sjamaan
Copy link
Contributor

sjamaan commented May 22, 2019

Please don't change indentation of existing code. This causes your patch to be larger than necessary and makes it hard to review. Ideally, you should configure your editor to accept editorconfig configuration, that would've prevented that in the first place.

Can you rebase your patch such that it doesn't create so large a change?

@ChandlerBent
Copy link
Author

It's done.

@sjamaan
Copy link
Contributor

sjamaan commented May 22, 2019

Thanks for the quick turnaround! I'll add a test app which will make it easier for me to test your code and get back to you (might take a couple of work days).

@sjamaan
Copy link
Contributor

sjamaan commented Jun 12, 2019

Sorry for the delay. I finally got around to this. Your code works, but I think it's a bit confusing, it looks more like a date and time stamp input (point in time) than a relative delta input (period of time). Especially due to the way time is separated from the years, days, months. Could you change this a bit to make it clearer that this is for interval of time rather than a specific point in time?

@ChandlerBent
Copy link
Author

I will think about that how to change it and make it be real relative delta input.
But Django DurationField is TextInput. I think other developer will set clear verbose_name and help_text for administrator to use.

Anyway, make it more easy and friendly for other developer to use is right.

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