search

FMCorz / mdk

Moodle Development Kit. A collection of tools meant to make developers' lives easier.
GNU General Public License v3.0
86 stars 47 forks source link

Add multiple new features to mdk #198

Closed dcai closed 3 years ago

dcai commented 3 years ago
  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
  3. Trigger behat tests on ci server image
  4. Allow pipe in alias config, for instance, you could have this: image
andrewnicols commented 3 years ago

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

FMCorz commented 3 years ago

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 commented 3 years ago

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

dcai commented 3 years ago

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

https://github.com/FMCorz/mdk/pull/199 https://github.com/FMCorz/mdk/pull/200 https://github.com/FMCorz/mdk/pull/201 https://github.com/FMCorz/mdk/pull/202 https://github.com/FMCorz/mdk/pull/203

FMCorz commented 3 years ago

@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.