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

Fixed 'Add tags'. Added 'Edit tags' #27

Closed
wants to merge 1 commit into from

Conversation

Theodorson
Copy link

I made a fix for "Add" which can cause problems in the current implementation, if we want to add tags for multiple issues.

Example:

  • first issue with tag1 tag2
  • second issue with tag3, tag4
  • we use "Add" tags and add the tag5
  • result => first issue will have tag1,tag2,tag3,tag4,tag5 and second issue same tags as first issue
  • in conclusion the "Add" behaves like "Set"
  • new implementation will just add tag5 for both issues
  • new result => first issue with tag1,tag2,tag5 and second issue tag3,tag4,tag5

New implementation contains "Add" which now behaves like an add and a new menu entry "Edit (for single issue)" which has same behavior like old "Add" implementation but just for a single issue (useful for easy add/delete tags for one issue).

New "Edit (for single issue)" (old "Add" but just for single issue).
image
image

New "Add".
image
image

@Theodorson Theodorson force-pushed the stable branch 3 times, most recently from 4351d55 to dfeea5f Compare August 2, 2022 09:53
@alexandermeindl
Copy link
Contributor

Hi @Theodorson,

you are right, with bulk operations adding tags would be better as changing them.

Maybe a better solution for bulk edit (form) would be, to provide one text field to add tags, and another text field to remove them. I am not sure, if it is required to change tags (replacing existing tags) in bulk edit (form).

To your PR:

  • I am not happy with the overwrite parameter, maybe there is a better solution without it?
  • there are no tests for the new behavior

@Theodorson
Copy link
Author

Hi @alexandermeindl ,

I can add tests for the new behavior. Can you please let me know what is your proposal for implementing without overwrite parameter ? I am not happy with this too. I tried to do other stuffs but finally I managed this.

@Theodorson
Copy link
Author

Hi @alexandermeindl,

Have you thought of an idea that would be better without the overwrite parameter ? I would like to modify the implementation and create tests for it.

@Theodorson
Copy link
Author

Hi @alexandermeindl,

I've changed the implementation transforming overwrite parameter to append. This means the new parameter is used just for this case and the existing tests weren't changed. I added tests for new behavior too.

@alexandermeindl
Copy link
Contributor

Hi @Theodorson,

this looks much better for me. I cannot merge your changes because you do not use main branch for your development. All development on this plugin takes place in main branch. If you provide you changes on main branch, I'll merge it after reviewing it.

@Theodorson
Copy link
Author

Hi @alexandermeindl ,

I've made changes on main branch and create this PR #39. After reviewing/merge it, the changes will be transfered to stable branch ?

@alexandermeindl
Copy link
Contributor

A new stable release can only be released, if additionals plugins is released, too. Most likely end of this year, a new release will be published.

I close this PR, because you created a new one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants