Create a new flair model

[PythonCoderAS]

As of right now, flairs are returned as dicts. I was thinking of making them into Flair objects.

Several questions:

• How do I make the Reddit instance objectify the dict into this flair object?
• Should I throw a ClientException when a user tries to edit a flair that it cannot, or just pass.
• Should the flair() method accept a flair object (current), or both a flair object or an id/text combo?

[kungming2]

This is going to break a lot of scripts, isn't that right? I know that this will definitely break my AssistantBOT.

My personal preference is to have some sort of backwards compatibility with the old system.

[PythonCoderAS]

This is going to break a lot of scripts, isn't that right? I know that this will definitely break my AssistantBOT.

My personal preference is to have some sort of backwards compatibility with the old system.

I'll make it a passable command and do a full migration on PRAW 7

[jarhill0]

Even though we have the ability to introduce breaking changes with major version bumps, I still think we should avoid them when possible.

As @kungming2 says, this seems like the sort of thing that would break a lot of scripts. In my view, the upside is not worth the downside.

[PythonCoderAS]

Even though we have the ability to introduce breaking changes with major version bumps, I still think we should avoid them when possible.

As @kunming2 says, this seems like the sort of thing that would break a lot of scripts. In my view, the upside is not worth the downside.

I'm making it backwards-compatible similar to a id/url system

[kungming2]

I guess the good thing is this shouldn't break the old flair() method which uses just CSS and text instead of the new templates. A lot of scripts still use this one instead of select(), which is the newer template-based and dictionary one.

I think it making flairs into objects does make it cleaner, but compatibility is something we don't want to move too fast on IMO.

[jarhill0]

I think it making flairs into objects does make it cleaner, but compatibility is something we don't want to move too fast on IMO.

Yep, I agree.

[PythonCoderAS]

I am trying to make this compatible with my other PR #1226 but I don't know if I should mix in mod methods with normal user methods

[bboe]

@PokestarFan can you reiterate what outstanding questions you still have? I concur that having flair as its own type might make sense now, but we definitely need to do it in a backwards compatible as default way first before breaking the existing functionality.

[PythonCoderAS]

@bboe as of right now I do not know how to make the reddit API return flair objects instead of dicts.

[PythonCoderAS]

This PR is ready. I think I have mostly everything done. As of right now, everything takes the old values and an additional flair argument

[PythonCoderAS]

Todo:

• Allow subreddit redditor flairs to be updated with flair objects
• Allow subreddit post flairs to be updated with flair objects
• Make UnitTest if find_advanced_flair_template does not find anything

[bboe]

@PythonCoderAS now might be a good time to learn how to interactive rebase your commits (if you don't already know how) to make them atomic. I prefer to review commit by commit, but that process only works if each commit is a small independent chunk. 85 commits doesn't work for that process, and 1 big squashed commit doesn't work either for how many changes this PR has. Also, if there is groundwork for this pull request that could be made into a smaller pull requests, that would optimize the time to get these changes in.

For example, a commit should include a subfeature along with all the tests for that subfeature. Should you need to make any edits, or subsequent formatting changes to said changes, they should be "fixed up" into that commit.

[PythonCoderAS]

I really do not understand how to use the rebase feature

[bboe]

Perhaps this video will help (I haven't watched it) https://www.youtube.com/watch?v=tukOm3Afd8s

[PythonCoderAS]

@bboe the video is for reordering commits and editing the commit message

[bboe]

You can follow similar steps to squash or fixup (merge two commits together). Also, you can use the edit feature for a single commit, but that's a little more advanced. Maybe search for some other videos.

[PythonCoderAS]

@bboe the new 9 commits are all of the older commits but combined into files changed

[PythonCoderAS]

I could not rebase these commits so I added them to the stack

[PythonCoderAS]

Please review #1226 before this PR as this is meant to complement that PR, although it does not rely on that PR going through.

[PythonCoderAS]

More TODO:

• Allow users to submit posts using flair classes

[kungming2]

Can I just say that I have no clue what the state of this pull is now? There's no documentation on this PR short of going through every single changed file on what the proposed changes are and as such it's impossible for reviewers to evaluate.

[PythonCoderAS]

@kungming2 check the changelog

[kungming2]

I think this has enough changes and flow alterations that if this is merged the version number should probably go up to 7.0. And the changelog doesn't really help with helping people who want/need to adapt their code to account for all these dramatic changes.

[PythonCoderAS]

I tried my best to make it compatible with current code so I don't understand what I need to change to make it completely compatible.

[kungming2]

You're guaranteeing then, that no one's scripts will break as a result of these changes? I still don't get the added value this gives other than dictionaries now being flair objects.

I don't mean to sound so negative on your work, which I'm sure is a lot, it's just this is a huge change for something people weren't exactly clamoring for.

[PythonCoderAS]

This is a huge change, but I went out of my way to make all existing scripts work. Everything is added in as an optional argument that defaults to false.

[bboe]

@PythonCoderAS given the size of this PR I practically don't think we'll be able to get it in as is given the amount of time that @jarhill0 and I have to review. If you think these changes are still worthwhile, I'd recommend, breaking it up into independent chunks that could be discussed / merged one at a time.