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

Exclude fields #274

Merged
merged 13 commits into from
Jun 12, 2017
Merged

Conversation

joaojunior
Copy link
Contributor

Create option to pass fields to not copy in history.

This new PR replaces the PR: #238

@codecov-io
Copy link

codecov-io commented Apr 15, 2017

Codecov Report

Merging #274 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #274      +/-   ##
==========================================
+ Coverage   97.54%   97.58%   +0.04%     
==========================================
  Files          13       13              
  Lines         570      580      +10     
  Branches       69       72       +3     
==========================================
+ Hits          556      566      +10     
  Misses          7        7              
  Partials        7        7
Impacted Files Coverage Δ
simple_history/models.py 96.85% <100%> (+0.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a1a23e...1f9d529. Read the comment docs.

@joaojunior
Copy link
Contributor Author

@gyermolenko @macro1 Hi, I send this PR to replaces the PR #238.
Here, I remove the mutable object in parameter of a function and I fix the error in tests with django 1.10

@joaojunior joaojunior mentioned this pull request Apr 15, 2017
@macro1
Copy link
Collaborator

macro1 commented Apr 26, 2017

Looks good. It would be great to have this documented as well. Undocumented features can be frustrating both for contributors as well as users of the library. If you don't have time to add the documentation, we'll try to get it added when this is merged in the future.

@joaojunior
Copy link
Contributor Author

@macro1 Very good! I will now write the docs for this.

@joaojunior joaojunior force-pushed the ExcludeAndIncludeFields branch from 3ffe151 to 3179eaf Compare May 11, 2017 19:13
@joaojunior
Copy link
Contributor Author

@macro1 I already add the documentation about this.

@vhelke
Copy link

vhelke commented May 23, 2017

When is this released? We are really looking forward to it. Cheers for the good work!

@leportella
Copy link
Collaborator

@joaojunior would you mind resolving the conflicts? Thank you!

Copy link
Member

@treyhunner treyhunner left a comment

Choose a reason for hiding this comment

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

This seems like a useful feature! I requested a couple small grammatical changes to the documentation.

👍 from me once those changes are made! 😄

Choosing fields to not be stored
--------------------------------

It is possible to use the parameter ``excluded_fields`` to choice which fields
Copy link
Member

Choose a reason for hiding this comment

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

choice -> choose here

question = models.CharField(max_length=200)
pub_date = models.DateTimeField('date published')

And you don't want to stored the changes for the field ``pub_date``, it is necessary to update the model to:
Copy link
Member

Choose a reason for hiding this comment

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

to stored the changes -> to store the changes

@treyhunner treyhunner requested a review from leportella June 4, 2017 22:36
@treyhunner treyhunner merged commit b5a650a into jazzband:master Jun 12, 2017
@rossmechanic rossmechanic mentioned this pull request Apr 6, 2018
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.

6 participants