-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add some documentation #6
Comments
Yes I agree mark down is widely used and a good one use. I see you use both formats at the moment. I have plenty of experience with SafeConfigParser, as a variation of ConfigParser from the same configparser library. So I can write instructions on how to use it. I imagine it's a file created by hand like this.
It could grow with more exchanges. What else are you planning to use the file for? |
Exactly. i was thinking that we could also put Assets and Currencies in there and for different exchanges. So from Luno I only want to get BTC/ZAR and from Bitfinex BTC,ETH in USD and EUR. So it would try .coinrc -> environment variable -> command line option. |
Ah yes that all makes sense. Oh and I see you also have |
I have improved the handling of the config file for the Luno keys (such as using config.has_section() And I actually implemented something on I'll write the documentation next and then docs and implementation in together. Here is what my .config file looks like.
|
Are you fine with the instructions to go in as a file called
I've implemented a config file approach for |
@MichaelCurrin @snth This will be much simpler, both to code against and to document. What do you guys say? |
@barrysteyn Sure. I'm not using or documenting the numismatic.ini file as thought it was unnecessary. Are you saying leave out the environment variables feature? That could be good to simplify code and instructions, both now and maintaining in future. But up to @snth. On the config file, I noticed that we can use exchange name as heading for setting assets and currencies for
|
@MichaelCurrin Yes, seeing as we can incorporate env variables inside the .coinrc, my suggestion is to leave it out all together. Makes the code easier to maintain, makes documentation easier as well. @snth - you have the final say, but what we could do is commit a config file that uses env variables for the secrets (like the API key) which allow it to work out the box in a container that could expose the variables, and it would also allow anyone to easily overwrite if they don't want to use env variables. |
Thanks for your comments. @MichaelCurrin I think exchange specific asset and currency options would be awesome. I'm keen to see how you have set this up. @MichaelCurrin @barrysteyn I think ConfigParser and click both provide ways for using a config setting or an environment variable. I haven't looked at these in detail so whatever is easiest. I'm happy for you guys to suggest something. @MichaelCurrin I think just submit your Pull Request and then we can discuss the different options for pulling in the environment variables. Your code doesn't have to be ready to merge yet. We can just use it as a base for discussion so that we are not discussing in a vacuum. Sorry about the |
@barrysteyn @snth Ok great. I've added my changes from the other day to a feature branch on my fork and did a tempory pull request to my own master, so you can see the changes. MichaelCurrin#1 Hopefully you can see enough from the diff there. Yes I find having a config file to view and edit a lot easier than managing what is in stored in Yes I agree that a versioned config file as a template with some base values would be good and it could be used to replaced the global defaults at the top of cli.py file. (This also makes it easier for a user to see what the default config options are) The approach I'm used to at work is as follows and may suitable here too:
|
In particular I would like to describe how to use the
~/.coinrc
file.What is the modern way of easily adding documentation? I would prefer to write in markdown rather than rst.
The text was updated successfully, but these errors were encountered: