-
Notifications
You must be signed in to change notification settings - Fork 66
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
Testcript enhancements #105
Conversation
I wonder - why did you not raise an issue about these features first? We generally discuss new APIs and features before going straight into code changes. For example, I don't think http really belongs in there. This is a generic test framework for Go commands in general, so http isn't really a necessity - and you can always add extra commands to do any custom logic you want. |
I also hadn't quite realised how large this pull request is. With such a large refactor and so much new code, I really do wish you had discussed this in the issue tracker first. For example, your |
@mvdan largely this is for visibility as I had not expected this repository to be that active. Many of the discussions have not seen movement in a long time (I did read them). I am personally using this package for testing non-go applications where this is extremely useful. I am able to simple drop a
It should be 100% backwards compatible |
I'm not sure if I follow. For example, that I appreciate that you put a lot of effort into this, I'm just noting that contributing with upstreams works best when the changes are coordinated and posted in small chunks. |
Well, it only took a couple of hours, so it's not really that much work, so three weeks does seem like a long time, give how easy this was. If changes of this nature are not desired, I can maintain a fork or more likely pull it into my growing monotool. Cuelang is something I really want to do in these scripts as well. |
The reason for the apparent "lack of progress" or activity on that issue (or indeed many others) is not really a function of people not having the time to write the code. Instead it's a function of spending time thinking about the solution.
As @mvdan has pointed out, there are two mechanisms by which the |
forget that I had PR'd anything... |
If you have any interest in following along hofstadter-io/hof#56 |
@mvdan @myitcv if this PR had been a "discussion" like we have on the Cue repo, would that be a better way to present something like this? i.e. Do you also think the new "discussions" feature would benefit from having creation options like:
There is a feedback link at the bottom of the Cue discussions page where I have submitted the above |
As @mvdan noted, an issue in the issue tracker would suffice as a starting point. If it makes sense to discuss code a PR could follow. Discussions is not GA yet, in any case I think an issue would be more appropriate |
This adds some requests in the issues / PRs plus a number of other things
what do you think?
Script upgrades:
~
for line comments (need to write messages for devs in test files)?
for ignoring exit codes, changes neg from bool to intstatus
for checking exit / status codeshttp
call with many args supported, also client management to support named default setups and multiple client connectionscall
allows users to add functions that act like commands without exec (useful for coverage)Config options:
Glob
param optionFuncs
param option for callPhasePrefix
for configuring phase indicatorCommentPrefix
for configuring the comment prefix