robert-scheck commented on PR #407: URL: https://github.com/apache/guacamole-server/pull/407#issuecomment-1368276926
> Please update `src/guacenc/man/guacenc.1.in` to document the new option. Other than that, LGTM and I'm happy with things as they stand. Oh, good point - will do that. > There are some other points I think are worth considering, but which are not deal-breaking: > > 1. Using a structure to cleanly and directly associate codec names with their suffixes, rather than parallel arrays. > 2. Including a human-readable description within said structure such that the "Supported codecs" output doesn't rely on the user knowing the meaning of each codec's internal name. As mentioned earlier, I'm not a C programmer, thus I'm not sure how such a structure would like (nor whether I'm able to actually implement it) - sorry. > 3. Avoiding logging a redundant `ERROR: Unsupported codec!` immediately after `ERROR: Invalid codec.`. What's your suggestion here? Just `Supported codecs:`? > 4. It might be better to include the supported codecs in the overall usage output, to avoid users having to guess at least one codec before they see a list of what's supported. Do you have any preference? Would `[-c <mpeg4|libvpx>]` instead be suitable? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
