-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feature: add optional support for moved in / out blips #310
feature: add optional support for moved in / out blips #310
Conversation
@devansh-sharma-tw @naveengenupuritw - appreciate your feedback re this feature proposal |
@marisahoenig - appreciate any feedback from the team re this PR 🙏 |
Thanks for raising this @setchy ! We're internally discussing on adding the moved in/out status as well (to keep BYOR inline with our Thoughtworks Radar). |
@devansh-sharma-tw - hope all is well. Just checking in to see if your team has had a chance to review and consider this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding some suggestions. I think it's worth adding one new field and letting users have the option to use it OR the isNew
field. Would love your thoughts @setchy
@marisahoenig - I have updated this PR to support a new, optional column/field called Accepted values are case-insensitive To demonstrate, I've updated my TW volume datasets, eg TW Volume 28. All existing datasets using |
@marisahoenig @devansh-sharma-tw - any feedback re: this PR? |
This seems like a very nice addition. A shame it's been sitting for so long |
Note In the interim I've deployed my BYOR enhancements @ https://radar.setchy.io, which bring it up to the functionality found on https://thoughtworks.com/radar |
@marisahoenig: Any plans or update on this one? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @setchy I left a few comments. Please resolve and also make sure the most recent code from the main branch of this repo is loaded in. I'll chat with the team next week to see if we can get this added. The slowdown was related to the most recent Radar release, and we need to make sure changes like this are reflected on our live site as well. Apologies for the delay — it's a great add to the tool.
Thanks @marisahoenig, appreciate the feedback. I've gone through and addressed each of them 😄. No problems about the delay, too - completely understand. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all the changes, @setchy! As mentioned, I'll chat with the team next week about getting this incorporated and seeing if there is anything else that needs to be updated on the team's end.
@will-amaral - appreciate your thoughts on this PR |
Thank you @shiviraj @marisahoenig @will-amaral - what a great gift for a Thursday morning 🙏 |
Hot on the heels of the v1.0.0 launch this week, I thought I'd see whether I could self-serve #177
Adds optional support for moved in and out blip drawing.
Below are two datasets for Thoughtworks Volume 27 sample (csv) that can be used to demonstrate/test: