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

WIP: add option to use --id in commands instead of --path #46

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rustysys-dev
Copy link
Contributor

Changes:

  • Changed to using struct AdrMeta to store path and id options
  • Changed to AdrMeta::build_from_meta for building ADRs
  • Added function get_path_by_id to get the path of an existing ADR
  • Added function is_id to check if a DirEntry is specified ADR id
  • Added --id parameter to adr lf new to specifiy a specific id on create_adr
  • Optimized some tests
  • Changed tests to use AdrMeta::build_from_meta
  • Updated cucumber to use AdrMeta struct

@rustysys-dev
Copy link
Contributor Author

@omallassi I think I should add some tests for AdrMeta and some tests for creating/transitioning ADRs using --id instead of --path

How do you think this should implemented? What should be cucumber tests, and which should use standard tests?

I suppose I am not entirely sure of the testing goals for this project yet.

@rustysys-dev rustysys-dev changed the title add option to use --id in commands instead of --path WIP: add option to use --id in commands instead of --path Jun 21, 2020
@rustysys-dev
Copy link
Contributor Author

I am also preparing another commit, because... I don't know how, but it seems I deleted one of the tests completely....

@omallassi
Copy link
Owner

@rustysys-dev
hope you are doing well! thanks for this.

regarding "How do you think this should implemented? What should be cucumber tests, and which should use standard tests? I suppose I am not entirely sure of the testing goals for this project yet."

Honestly, we are not at this level yet so we can clarify this w/ pleasure :)

In my mind :

  • the rust test (ie. unit test) where much more to test a single method, algorithm etc...
  • the cucumber test where much more acting with scenario from an end-user perspective : As .... I want to create and ADR etc...

so in your case, maybe this is a mix of the two types of tests:

  • maybe the get_path_by_id should be unit tested (so you can "validate" all combination, with nested folders etc...)
  • maybe the rest (commands based on --id) should be in cucumber test

what are your thoughts on this?

@rustysys-dev
Copy link
Contributor Author

@omallassi
Sorry for the delay, I am doing well! a bit exhausted (work has been long lately) but otherwise good.
with CORVID around hopefully you are well too?

Regarding the tests I agree, I think both will be necessary, and I can see a lot of new cucumber scenarios where the user creates/modifies ADRs using ID numbers. I have never really used a framework like cucumber, so it will be an interesting learning experiance!!

I don't have much time right now during the weekdays due to needing to build a couple of new microservices at work, but I plan to get to this on the weekend.

I will let you know if I run into anything that might be tricky. 😆

@omallassi
Copy link
Owner

@rustysys-dev - let me know if I can be of any help : writing some cucumber scenario etc...

@omallassi
Copy link
Owner

@rustysys-dev - hope you are doing well.
did you get some time on this? do U think we can merge it with the current master?

cheers.

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.

2 participants