-
Notifications
You must be signed in to change notification settings - Fork 32
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
Remove reference to commit IDs #622
Conversation
Then link still points towards https://github.com/open-source-economics/Tax-Calculator. How do I edit the html code so that it points towards: In Python, I would do something like this:
|
@hdoupe asked:
I don't think this needs to be an HTML link does it? |
@martinholmer I think that the link should point towards the version that the webapp is using. This PR shows the version of the Tax-Calculator that TaxBrain is using but when you click the link it takes you to instead of https://github.com/open-source-economics/Tax-Calculator/tree/0.9.0 |
@hdoupe said:
OK. If you have the version/release number (for example, Do you know where in the code the variable |
@hdoupe, #622 is good progress, but can we add just two more things? First, everywhere we have the phase "created_on|date", can we add the time zone, And second, can the sentence containing the "created_on|date" phrase, read like this?
I think you can get the TaxBrain version by calling |
@martinholmer Sounds good to me. I'll add these suggestions. |
@martinholmer said
This seems sensible to me but I got the following result: I think there may be an issue with how the version is set. Lines 21-24 of taxbrain_server/_version.py are
and But in
and
I thought that maybe you had to run
and
However, when I tried to retrieve the version:
But when I do the same thing for Tax-Calculator, I get:
It looks like |
@brittainhard, How does one get the version of TaxBrain (that is, the webapp-public release number, 1.y.z)? |
@martinholmer i'll take a look. |
Thanks @brittainhard |
@hdoupe @martinholmer there are a couple of solutions to this. Since TaxBrain isn't a python package, the only way to know its version is by getting the latest git tag. In the app, you would have to make a subprocess call like this:
However, I think it is a better idea to set a string constant variable that provides the version. In the settings file you could add Since the webapp version isn't something that changes often, it's not worth it to make a programmatic call to get the current version every time a page is loaded. I suggest going with the second option. Let me know if you have any questions. @martinholmer I appreciate the ping on this one. There's lots of stuff to keep track of, and some things just slip through the cracks. |
@brittainhard said:
Thanks for the helpful information. But I have one question. If versioneer doesn't provide the TaxBrain version, what does it do in webapp-public? Put another way: Why don't we eliminate versioneer from the webapp-public repo? |
@martinholmer I know there is a versioneer script for the deploy folder (since it used to be a separate repository) but I don't think there is one for webapp. In that case its probably wise to remove the deploy versioneer script. I think that since there is no standard release process for webapp (just version tagging with a string) there wouldn't be much use for versioneer. EDIT: By "standard release process" I mean a release process that uses conda or pypi and a setup.py script. |
@brittainhard said:
@brittainhard, Thanks for the explanation. |
Thanks @martinholmer and @brittainhard |
@martinholmer @MattHJensen Do you think that we should use "Webapp-Public version 1.0.1" or |
@hdoupe asked:
Good points. However, I don't think anybody but the developers will understand "Webapp-public". I assume that in your question, you meant to refer to 1.0.2, right? Isn't that the next webapp version that will be using taxcalc 0.10.2 (or 0.10.x if you find more taxcalc bugs)? |
That makes sense. I'm going with "Tax-Calculator." I'll use "Webapp" unless @MattHJensen objects. We could change it in the future, too.
Yep, 1.0.2 is the right version. Nice catch. |
@hdoupe, my preference is "PolicyBrain." I also plan to rename the repo, and eventually we ought to separate ospc.org content to a separate repo. |
@MattHJensen Ok sounds good to me. |
I just changed the repo name 💥 |
@MattHJensen Awesome, I'll make the path changes in this PR, too. |
Thanks. All of the old paths will remain active, so I do not believe that the update is urgent. But it is certainly good to do. |
@MattHJensen Ah, right. Never mind. |
@brittainhard @GoFroggyRun This PR is ready for review. |
Nevermind, I'll take a look at this. |
Ok, now this is ready for review. |
@GoFroggyRun Thanks for the review. I'll take a look at this. |
LGTM, merging. |
This PR removes the reference to the commit ID for
taxcalc
,ogusa
andbtax
as requested in #612. Since all of these packages are versioned, there is no need to pin them to the version and specific commit.