The if could be moved within the <option> tag to save a bit on space.

Are all the non-breakable spaces needed?

I wonder if it would be worth doing a loop over this collection of if/else

@lsedlar proposed a nice change recently using the .clone() method in JS, might be worth investigating here as well

most often for these I prefer adding something to pagure.lib than calling the model directly

Sorry I don't understand what you mean. Do you want me to add a function to init.py that returns a new tag object? Seems redundant.

I added the nbsp's so the color combobox would have width. Otherwise it's really really tiny. I would love to get rid of the nbsp's. but I couldn't find another way to do it. I'm open to suggestions.

I will also look into doing this in a loop.

Maybe with CSS we can get ride of the nbsp?

I applied all the changes (see new commit) except the js.clone. Sorry I'm not sure how to apply this "clone" change.

I'm not seeing a table named tag_id in model.py, did you mean tags?

Could we move the query into pagure.lib?

As much as possible I do not want the web-controller to have any interaction with the model. It makes the maintenance of the application easier, as changing framework or the ORM would be "simply" changing the corresponding part of the application :)

We'll need to adjust the docstring to what we want

I would put the color under the issue_uid so that we have the composed primary key at the start of the table definition.

We probably want to use tag_id here rather than tag

Any reason for this change?

This could like be made a for loop

What about putting these in the configuration since we seem to also be using this list in the templates?

Ok, this is pagure.lib, I'm out :D

We could add the color here, it could help explain what was updated.

I have been a little wondering if we shouldn't make this a plain integer, what do we gain with having it be a string?

from tag we cannot access its color directly?

Nope we can't it's not an object.

But then we're iterating over the list of tags twice, maybe we can optimize this

Since we have pagure.lib.new_tag() shouldn't we use it here as well?

I really need to turn my keyboard 7 times in my mouth before asking question (and check the file of the diff I'm looking at).

Code says commit, comment says flush?

None of this is tested. Sorry I thought I stated this needed to be properly finished. I was going to ask you for help on this because its a bit complicated

We'll need to adjust this to 588eabcd394c

Sounds good, but I don't know where the "configuration" is stored

We'll need to adjust this to 588eabcd394c otherwise we end up with two revisions revising the same one, (ie two heads), you can see this if you rebase and run alembic history the last two revisions claim to revise the same one.

I'll fix this

Sorry I don't follow what you are suggesting

I remembered you needed help to finish, but since I saw some commands I thought there were working and we just needed to finish the migration part.

It's not a problem, we'll get it done. Let's not worry about the migration yet and concentrate on getting the model as we want :)

pagure/default_config.py can then be accessed with pagure.APP.config

It can be anything as long as its unique. Using the tag name and project ID made sense to me. What would you suggest that is simple and clean?

this is sending a notification about a tag being edited, we should include the new and old tag color as otherwise we could have a notification saying that foo was edited to foo which lacks a little bit of information :)

What most id are: simple integers the database will take on itself to increment it as needed

Okay, I had no idea the order made a difference.

Nope, not any more. I'll get this cleaned up.

Does that mean we no longer support renaming tags?

I doesn't per say, it's more a personal preference to have the primary key (both fields in this case) listed at the top

Renaming still works. I need to look at this test again.

Ahh I see. I didn't really know what/how the notify process worked. I'll work on this as well.

I'm not sure that is needed, since the tag issue is already part of a specific project/issue. But it won't hurt so I'll look into changing this - its going to impact more code though.

So just changing sa.String to sa.Integer will cause the db to automatically increment it? How does that work? I must be missing something. :)

on postgresql, sqlalchemy will create a sequence that will be called upon inserting a new line and that sequence will take care of incrementing the id for each insert :)

Made two commits that should address everything. FYI, I will be out Monday