Time
S
Nick
Message
02:42 jri joined #dataverse
05:42 jri joined #dataverse
08:35 jri joined #dataverse
10:35 ruben48
pdurbin I got Dataporten working now, code here: https://github.com/uit-no/dataverse/tree/dataporten
10:36 ruben48
one thing I noticed is that no mather with OAuto provider I use, the Log In button always displays "Create or Connect your ORCDI"
10:37 ruben48
which OAuth*
11:06 pdurbin
ruben48: great that you got it working! There's a known issue with that button, just reported yesterday: https://github.com/IQSS/dataverse/issues/4333
11:09 pdurbin
If you go to https://dataverse.harvard.edu and click "Log In", you can see the bug is there too. I haven't looking into it yet but probably will today. If you want to try to figure out what's going on, I'd appreciate it.
11:10 ruben48
it's more or less hardcoded in the template loginpage.chtml
11:10 ruben48
loginpage.xhtml
11:11 pdurbin
Yeah, `login.button.orcid` apparently.
11:11 ruben48
jepp
11:15 pdurbin
If you look at https://github.com/IQSS/dataverse/commit/11d4f0dd95d539889e125af2ea614c5282c53deb#diff-1edaf9d6e72c76a445255cf6a4780a47 you can see that #{LoginPage.loginButtonText} was replaced by #{bundle['login.button.orcid']}
11:15 pdurbin
ruben48: do you want to see if that fixes it?
11:16 ruben48
yeah sure, I can do that
11:16 ruben48
about Dataporten, will you consider a merge request? It will be useful for all the higher education insitusions in Norway
11:24 pdurbin
ruben48: well, over in th Google Groups thread, you may have seen me write this: "I'm not sure how we'd even test your code without you giving us credentials to your national OAuth provider. I get slightly nervous about having code in our source tree that we aren't sure works or aren't sure will work after we refactor related code in the future.
11:25 pdurbin
https://groups.google.com/d/msg/dataverse-community/pBJVjc4_wvs/jM_c3cNEBwAJ
11:25 ruben48
yes, I say that
11:25 ruben48
maybe we could provide some guest accounts if it helps
11:25 ruben48
saw*
11:26 pdurbin
I'll tell you what.... Can you please start by creating a GitHub issue?
11:26 pdurbin
That way we'll have something to talk about. That's why I wrote this: "We do all of our estimation by talking about specific GitHub issues, so in a future backlog grooming meeting, perhaps the IQSS team can talk about how much work on our part it would be."
11:27 pdurbin
We estimate GitHub issues on Wednesdays.
11:27 pdurbin
Including details like "we can provide a guest account" in the GitHub issue will mean the estimate will be lower. Less work for QA.
11:28 pdurbin
Also, I'd me remiss if I didn't point out what Gustavo wrote about modularity here: https://groups.google.com/d/msg/dataverse-community/pBJVjc4_wvs/8fVRMttXBwAJ
11:28 pdurbin
be*
11:29 ruben48
yeah, hoping for modularity :)
11:29 ruben48
but you know ho it is.. someone is paying someone so that I can work on this for a given amount of time
11:32 pdurbin
Sure. And of course, I don't want you to have to run a fork.
11:32 pdurbin
Without modularity already in place, you'd have to run a fork.
11:33 ruben48
yeah.. and I'm not comfortable doing that.. need to set up som CI to make deployment easier and probably a lot of work doing code reviews
11:33 pdurbin
I'm going to sit and have breakfast with the family. Back in a bit.
11:34 ruben48
Have fun, I will be at work for another 2.5 hours
11:58 pdurbin
ruben48: the best thing you could do is create a GitHub issue. You can either present it as an issue or a suggestion. You can explain the benefit (more people using Dataverse) and you can say that you are happy to write the code and provide a means for the Dataverse team to QA the solution (a future pull request).
11:59 pdurbin
That way, the team at IQSS will have something on the screen to estimate. We'll give it a 2, 3, 5 or whatever based on what you write in the issue.
12:00 pdurbin
Heh, I was going to say that it will appear in the Inbox at https://waffle.io/IQSS/dataverse
12:00 pdurbin
but it's already there! thanks!
12:01 pdurbin
https://github.com/IQSS/dataverse/issues/4334 looks perfect. If you can follow up once you know about the guest account, that would be great. Otherwise, we won't be able to test the code.
12:14 ruben48
yeah, I just need to check that I'm in the clear if I provide a client id and secret with a few guest accounts
12:14 ruben48
need to talk up a level or two in the organisation :)
12:16 ruben48
about the text on the login button, seems like there is a function getLoginButtonText in LoginPage.java line 266 thats not on use
12:17 pdurbin
ruben48: do you feel like making a pull request for just that one line fix?
12:17 pdurbin
(I *think* it's just a one line fix.)
12:18 ruben48
shure.. need to get som goodwill in the community :)
12:18 ruben48
anyways ;)
12:19 pdurbin
heh, you've already got goodwill :)
12:19 pdurbin
I'm just glad the bug was reported by a close collaborator before you found it. :)
12:19 pdurbin
Only by 9 hours!
12:22 ruben48
only because I was working in that area :)
12:25 pdurbin
ruben48: I just invited you to https://github.com/orgs/IQSS/teams/dataverse-readonly/members so I can assign https://github.com/IQSS/dataverse/issues/4333 to you, if you want to be assigned. :)
12:42 ruben48
nice
12:42 ruben48
just fixed and testet the issue.. I love oneliners :D
12:43 ruben48
https://github.com/uit-no/dataverse/tree/4333-oauth-login-button-text
12:46 pdurbin
ruben48: great! Can you please create a pull request and use our crazy "connects to #4333" syntax? For an example, please see https://github.com/IQSS/dataverse/pull/4331
12:47 pdurbin
The reason we do this is so that pull requests are associates with issues over at https://waffle.io/IQSS/dataverse
12:47 pdurbin
associated*
12:57 pdurbin
We mention the "connects to" syntax at http://guides.dataverse.org/en/4.8.3/developers/version-control.html#make-a-pull-request
12:59 ruben48
holy shit, I think I got it wrong
13:00 ruben48
So it should be like Fixed login buttin text [ref: #4333] ?
13:08 pdurbin
ruben48: sorry, I'm talking about the *description* of the pull request. I'm not talking about the commit message. It's confusing.
13:11 pdurbin
Right now, the description says, "Replaced a hardcoded string with an already existing function to get the login button name." You would edit this text and add "connects to #4333" so that Waffle knows which issue to show the pull request under. It's a Waffle thing. I can dig up their article on this if you want.
13:11 pdurbin
Here, it was easy to find: https://help.waffle.io/faq/waffle-workflow/use-waffles-connect-keyword-to-connect-prs-to-issues
13:12 pdurbin
"Example: If your Pull Request description for PR#19 was "This work connects to #34", this would result in PR#19 being visually connected underneath issue #34."
13:13 pdurbin
"visually connected" is the important thing to us, so that when we stare at waffle at standup every morning we can see a single card moving across the board
13:13 pdurbin
I hope this makes sense.
13:15 pdurbin
ruben48: ah, you fixed it. Thanks. Next question... If you log in to https://waffle.io/IQSS/dataverse are you able to drag your card into the "Code Review" column?
13:16 pdurbin
Do you have permission, I mean, since I added you to that "read only" team.
13:17 pdurbin
If not, please let me know and I'll drag it over for you. I explain this at http://guides.dataverse.org/en/4.8.3/developers/version-control.html#make-sure-your-pull-request-has-been-advanced-to-code-review
13:18 ruben48
no, I get an error " Only collaborators can update cards."
13:20 pdurbin
ruben48: ok, that's what I expected. Thanks for testing this for me. I dragged it over for you. Thanks for the pull request! We can take it from here. :)
13:20 ruben48
nice :)
13:22 ruben48
I have started to digg into what is possible for me about giving out testing information against Dataporten
13:22 ruben48
I will probably have the answer early next week
13:28 pdurbin
ruben48: awesome. Do you have time for a little code review here or do you want me to put it in as a formal code review?
13:29 pdurbin
The first thing I'll say is thank you again. I believe this will fix the problem. As a code reviewer, I don't feel the need to actually execute your code. You said you tested it and I believe you. QA will test it before merging. But there are a few little things I'll mention.
13:31 pdurbin
Since this is a one-line change, ideally there would only be one commit with a single change to the line. Not a big deal. If you had created a branch from the tip of the `develop` branch, we wouldn't see so many merge commits in your pull request.
13:32 ruben48
yeah, i noticed that
13:33 pdurbin
Also, I don't see your GitHub avatar (profile icon) on your commits. This means that whatever email address you have configured in your git client doesn't match the email address you use on GitHub. This means you won't appear as a contributor under https://github.com/IQSS/dataverse/graphs/contributors if that matters to you.
13:33 pdurbin
I'd love to have you be #50. :)
13:34 pdurbin
I think that's it. I'll leave it up to you if you want to create a fresh pull request with those two things in mind. I'm sure this won't be QA's for another three hours or so.
13:34 pdurbin
QA'ed*
13:35 pdurbin
You can associate multiple email addresses with your GitHub account, by the way. I do this for work vs. personal projects.
13:35 pdurbin
if you do `git log` you'll be able to see what email address was put in
13:36 pdurbin
some people don't care about the "credit" GitHub gives you
13:36 pdurbin
I think it's kind of fun. GitHub's original tagline was "social coding".
13:44 pdurbin
Either way, I'll try to make sure you get thanked in the release notes. :)
13:50 ruben48
I tried again :)
13:52 pdurbin
ruben48: much better! You tested it? It seems to be the same change.
13:52 ruben48
Also, I have got a GO on providing you all you need to test OAuth against Dataporten. Just need to know the Redirect URI to your test environment, but we can take the details on Monday
13:53 pdurbin
ah, great news
13:53 ruben48
yes, I tested it. Same change, just with the changes you pointed out like the head of develop branch and my correct email
13:54 pdurbin
Thanks for testing. For the redirect url, can you please leave a comment in the issue you opened?
13:56 pdurbin
And if you remove the "connects to" from your old pull request, the waffle card will be less confusing for QA. Closed pull requests are greyed out, but it's a little subtle. Better to disassociate it completely.
13:59 ruben48
Done
14:00 ruben48
I'm going off work now. Talk to you next week :)
14:00 pdurbin
Thanks again! Have a great weekend!
14:01 ruben48
you too!
15:20 pameyer joined #dataverse
15:26 soU joined #dataverse
15:33 soU
Hi pdurbin: as we discussed last time its just to inform you that our dataverse goes live at data.cimmyt.org.
15:46 pdurbin
soU: congrats on a successful upgrade to Dataverse 4!
15:47 soU
Thanks, And thank you the whole community for the amazing support we got
15:47 pdurbin
You're quite welcome. Are you running a fork? If so, is the code on GitHub? I track forks internally in a Google doc.
15:50 soU
https://github.com/solhm/dataverse
15:55 pdurbin
Thanks, it looks like this one is the most recent: https://github.com/solhm/dataverse/tree/3.6to4.xmigration
17:05 jri joined #dataverse
17:07 matthew46 joined #dataverse
17:09 pdurbin
So! pameyer was able to push to ruben48 's https://github.com/IQSS/dataverse/pull/4336 thanks to the magic of a new GitHub feature: https://help.github.com/articles/committing-changes-to-a-pull-request-branch-created-from-a-fork/
17:10 pdurbin
Thank you, ruben48 for allowing us to push to your fork. This is going to make it easier to make a few tweaks and git this pull request to QA.
17:12 pdurbin
Those instructions from GitHub are great, but I'm going to add ruben48 's fork as a "remote" and see if I don't need to re-clone.
17:13 pdurbin
If you type `git remote` you can see all of your remotes. By default, you should only have one remote called "origin".
17:15 pdurbin
git remote add uit-no git
17:15 pdurbin
git fetch uit-no
17:16 pdurbin
git checkout 4333-login-button-text
17:16 pdurbin
Ok, now I'm on ruben48 's branch and I can see pameyer 's commit. Let's take a look.
17:18 pdurbin
pameyer: your commit makes more sense to me now. You don't like `login.button=Log In with {0}`, right?
17:24 pdurbin
Before making any changes, I'm re-reading what I wrote at http://guides.dataverse.org/en/4.8.3/developers/dev-environment.html#shibboleth-and-oauth :)
17:29 pdurbin
Hmm, what I wrote there isn't very useful for this. What I really need is to add some auth providers, as discussed at http://guides.dataverse.org/en/4.8.3/installation/oauth2.html
17:38 mdunlap joined #dataverse
17:48 pdurbin
pameyer: I'm warming up to your fix. :)
17:58 jri joined #dataverse
17:59 pdurbin
Yeah, I'm just going to add a comment for future developers.
18:00 pdurbin
Normal commit.. then...
18:00 pdurbin
git push uit-no 4333-login-button-text
18:01 pdurbin
pameyer mdunlap if you're interested, that's how you can push to a branch on a fork without having to re-clone the entire repo. Note the `git remote add` and `git fetch`` and `git checkout` above as well.
18:13 pameyer
a little bit of lag from my end - but yeah, the issue was changing "log in with $x" to "connect $x" in some cases
18:13 pameyer
and I think that's what went wrong
18:14 pdurbin
Yep. Oh, that reminds me. Do we know when the bug was introduced?
18:14 pameyer
I haven't checked to confirm, but I'd guess when orcid 2.0 support was merged
18:14 pameyer
because that's when it was originally changed
18:15 pdurbin
Let me go look at the commit. It'll show which tags/versions the regression is in.
18:17 pdurbin
Only 4.8.3 is affected. Otherwise, more tags/versions would be at the top of https://github.com/IQSS/dataverse/commit/11d4f0d
21:39 pdurbin
Bah, there's a problem with Google login (unrelated to ruben48 and pameyer 's pull request, I'm sure). I just opened https://github.com/IQSS/dataverse/issues/4337
21:40 pdurbin
I gotta run. Have a good weekend, everybody.
21:40 pdurbin left #dataverse
