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

Add multiple new features to mdk #198

Closed
wants to merge 6 commits into from
Closed

Add multiple new features to mdk #198

wants to merge 6 commits into from

Conversation

dcai
Copy link
Contributor

@dcai dcai commented Jun 15, 2021

  1. Add ability to pull jira comments, highlight jira text and emoji support, color can be turned off by --no-colors

image

  1. Add ability to open current jira issue in default web browser: mdk tracker --open
  2. Add ability to customise logging format and add colors to differentiate log level

image

4. Trigger behat tests on ci server

image

5. Allow pipe in alias config, for instance, you could have this:

image

@andrewnicols
Copy link
Contributor

I'd suggest breakig these out into separate pull requests - one per feature.

@FMCorz
Copy link
Owner

FMCorz commented Jun 15, 2021

Hi @dcai,

Good to see you around here! Thanks for these improvements. I've not tested them, but here are a few comments, I don't see any of them as big deals, but rather clean-ups.

  1. Commits start with a capital, and do not include "Feature:" or the type of commit it is.
  2. Could we move the Tracker emojis to a Python file instead of making them configurable?
  3. What do you think of moving the new utils (ansi_escape_codes, add_color_to_log_record) into tools.py instead of creating new files?
  4. Instead of letting the whole log format be customisable, I would suggest using a config logging.colors: true/false. Developers without a good understanding of the logging module would not know how to use the logger format. This also makes it simpler to disable colours at a glance.
  5. You could do the same for tracker.colors and switch to --[no-]colors flags to reverse the default config. Also, I would suggest setting self.no_colors in run instead of info.
  6. On that note, would you anticipate issues in existing installations when switching to colours by default? If so, perhaps a global colors config is better than individual ones for logging and tracker.
  7. Could we avoid changing the order of the arguments in mdk/ci.py#init? That shouldn't matter, but it's best to add username as the last argument.
  8. I feel that the argment --number does not clearly indicates that this limits the number of comments. Would it be possible to have something like --comments [n] instead? Or --comments-count.

Perhaps @andrewnicols and @junpataleta have more comments, especially regarding internal tools (CI?).

Thanks!
Fred

@dcai
Copy link
Contributor Author

dcai commented Jun 15, 2021

@FMCorz Thanks, I'm breaking this into separate PR and going to address your feedbacks there.

@dcai
Copy link
Contributor Author

dcai commented Jun 15, 2021

I'm closing this PR as I have created smaller PR:

#199
#200
#201
#202
#203

@dcai dcai closed this Jun 15, 2021
@FMCorz
Copy link
Owner

FMCorz commented Jun 21, 2021

@dcai Thanks for creating those. Could you comment in each of them when you've addressed the comments above? I might be mistaken, but I haven't noticed any changes yet.

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.

3 participants