Skip to content
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

Od fixes #34

Closed
wants to merge 2 commits into from
Closed

Od fixes #34

wants to merge 2 commits into from

Conversation

jongough
Copy link
Contributor

Updates for Travis builds

@leamas
Copy link
Contributor

leamas commented Sep 15, 2019

Oops... collision ahead. Let's see who will have to rebase their branch.

@rgleason
Copy link
Contributor

rgleason commented Sep 15, 2019

Leamas,
I forgot to mention, that the whole purpose of the squiddio_pi od_fixes is to start using the Ocpn_Draw Textpoints which were specifically designed by Jon to show Squiddio icon properties. What this option does is move the squiddio icons out of ocpn navigation Waypoints which is a very big improvement. We were trying to get this plugin out to the public as a release, because it is quite stable. I think we can do that now with the v1.0.8 build on my repository. Jon has improved the CMake files and versioning, which is likely to collide with your work. Then what do we do about all your commits which will also be needed?

Leamas,
Jon did a big cleanup and improvement for many of the front end cmake files, scripts, and versioning which mirrors the changes he made in my tactics_pi repository and also his ocpn_draw plugin. He broke travis down into scripts and made the naming better. I believe he also added "opencpn_" to the front, which makes sense to me. I hope you will take a good look at those changes, because I think he took out dead stuff and tried to get back in shape.

I know you have many great improvements to these files too so we can have a more seamless process, So between your and Jon's work we will probably have the best! I think Jon is taking a break for a short whille, but he will definitely be interested in the plugin installer for ocpn_draw too.

@rgleason
Copy link
Contributor

rgleason commented Sep 15, 2019

I will refrain from making any more changes, until you are done.
There is one small icon matter "Boat Yards" which is not showing right now in all versions of squiddio, which requires adding appropriate code, but that can wait! I won't be adding those changes yet.
(If I get a chance, I will work locally in a separate branch and wait until you are done.

@rgleason
Copy link
Contributor

rgleason commented Sep 15, 2019

Leamas, I think your changes need to go on top of Jon's probably, but would you take a careful look at what he has done to clean up the CMake files?

BTW Mauroc and Jon have been working on this for quite some time. The reason the travis & appveyor compiling version is on my repos, is Jon did not want to host it on his, he tought it would be confusing I think, but he wanted to get it out to the public and released. That was what we were working towards. I believe v1.0.8 is that version. See the release here https://github.com/rgleason/squiddio_pi/releases
that was created by pushing a new tag.

@leamas
Copy link
Contributor

leamas commented Sep 16, 2019

@mauroc : Long story short I now have to rebase my changes on top of current master. This is gonna be some work for sure, but I actually think I would have done the same if I was in your position. Stay tuned...

@jongough
Copy link
Contributor Author

jongough commented Sep 16, 2019 via email

@rgleason
Copy link
Contributor

rgleason commented Sep 16, 2019

Sorry Leamas about the added work. If I can help in any way let me know.
What a great idea Jon, a variable driven setup in CMakeLists.txt!
How close are we to that? It seems like many of the variables are there now.

@jongough
Copy link
Contributor Author

jongough commented Sep 16, 2019 via email

@rgleason
Copy link
Contributor

rgleason commented Sep 16, 2019

I agree completely. The thing I can't understand is why sets version_major to plugin_version_major and then sets it to cpack_version_major and so on. Why not set the variable and keep it the same? My head gets spinning! There must be some historical reason for this that I don't know, but I think it should be simplified if at all possible.

@leamas
Copy link
Contributor

leamas commented Sep 16, 2019

I'm happy as long as I can rebase on current tip and get it merged before we make another roud of changes...

@leamas
Copy link
Contributor

leamas commented Sep 16, 2019

Long story short: until @mauroc actually merges one of the two open PRs we're stuck.

Of course, I prefer if my PR #35 is merged. That said, I could probably rebase it on top of #34 if that is merged first. But for now there is not much to do, it's @mauroc's show.,

@mauroc
Copy link
Owner

mauroc commented Sep 18, 2019

I just merged #35. Does that solve the problem? Sorry it's taking me a while to react. Been busy on another issue for the last few days.

@leamas
Copy link
Contributor

leamas commented Sep 18, 2019

Yes, we have a decision, and that's fine.

Now, since you merged my PR first @jongough needs to rebase his work on top of mine. In this context it's worth to note how we handled the ci clashes in the radar_pi plugin where we basically left the travis build chain as-is while the new artifacts is produced by the circleci builds.

@jongough : please feel free to do whatever you need to do with .travis, as long as the .circleci builds is left unchanged. The remaining ci issue is appveyor. Here, we basically need to have a common build but perhaps with two different deployment parts. It turned out to be a no-brainer in the radar_pi case; I'd assume the situation is the same here.

@mauroc: We need to discuss how we can boot up the circleci builds from your repo. This is about transferring the responsibility for the plugins installer artifacts to you. Please let me know if/when you are interested in such a discussion.

@mauroc
Copy link
Owner

mauroc commented Sep 24, 2019

Yes, we have a decision, and that's fine.

Now, since you merged my PR first @jongough needs to rebase his work on top of mine. In this context it's worth to note how we handled the ci clashes in the radar_pi plugin where we basically left the travis build chain as-is while the new artifacts is produced by the circleci builds.

@jongough : please feel free to do whatever you need to do with .travis, as long as the .circleci builds is left unchanged. The remaining ci issue is appveyor. Here, we basically need to have a common build but perhaps with two different deployment parts. It turned out to be a no-brainer in the radar_pi case; I'd assume the situation is the same here.

@mauroc: We need to discuss how we can boot up the circleci builds from your repo. This is about transferring the responsibility for the plugins installer artifacts to you. Please let me know if/when you are interested in such a discussion.

@leamas I am back from a long sailing hiatus and have some time to work on this. My understanding of CI is limited so I will need a bit of help. I am available to discuss this when you are ready

@jongough
Copy link
Contributor Author

I am away at the moment so will not be able to get to this for the next couple if weeks.
Is the Travis and appveyer integration working in this repository?

@rgleason
Copy link
Contributor

Jon, I don't know if this helps, but travis and appveyor does work in this repository.
https://github.com/rgleason/squiddio_pi/commits/od_fixes

Incidentally, we commented out line 45 of the ci / osx script, as Ron suggested, as a result he could install the mac pkg.

@jongough
Copy link
Contributor Author

I put the PKG stuff back in as OCPN only generates DMG packages from their version of this. With my change you 'should' get both. I am sure it did with my last changes, but I have to make changes to match the current master before I can verify that and I cannot do that for at least two weeks.

@rgleason
Copy link
Contributor

Mauro, if you would like to get appveyor and travis working, there is a write up for how to do this
https://opencpn.org/wiki/dokuwiki/doku.php?id=opencpn:developer_manual:ci-push-build-to-git
You will need a free opensource account with travis-ci and appveyor to do this use your git account as it helps with integration with git.
If you have questions Jon or I can help.

@rgleason
Copy link
Contributor

rgleason commented Sep 24, 2019

Jon, You'll need to look at it in two weeks. I think we might need if, then statement. There is no rush. Thanks.

@jongough
Copy link
Contributor Author

Is there a problem having both? They are both installable but they appear to have different capabilities. Perhaps OCPN should set the direction and plugins follow that. I don't mind as I don't use Marcos (except for testing). I am happy to go with what is decided.

@rgleason
Copy link
Contributor

Leamas wrote:

The remaining ci issue is appveyor. Here, we basically need to have a common build but perhaps with two different deployment parts. It turned out to be a no-brainer in the radar_pi case; I'd assume the situation is the same here.

Possibly found here
opencpn-radar-pi/radar_pi#120

@mauroc
Copy link
Owner

mauroc commented Sep 25, 2019 via email

@mauroc
Copy link
Owner

mauroc commented Sep 27, 2019

I have set up a travis-ci.org account and was able to successfully build the 3 linux distis + OSX using the .travis.yml file I believe @leamas had put in the mauroc/squiddio_pi repo. However, I am confused as to the proper place to deploy the packages. The .travis.yml file deploys to cloudsmith and Bintray (though I don't have the api keys) while this article talks about deploying to github. Someone has also mentioned Launchpad....
How is the actual deployment of plugins supposed to work?

@rgleason
Copy link
Contributor

rgleason commented Sep 27, 2019

Mauro, Congratulations. You have Travis and Appveyor accounts now?
At any rate it, Alex Leamas "Plugin Installer" appears to be working.

As you may recall, eariler I said there were two separate deployment approaches.

  1. The current, largely manual approach that deploys multiple OS to a github Release Tab when you git push origin [tag] Then these files get linked to on the Downloads website by K and Dave.

  2. Alec Leamas "Plugin Installer" deployment.

Because you accepted Alec's PR, travis and appveyor got changed to deploy to clloudsmith.
I am trying to get appveyor and travis to do both jobs, so that we can continue the current deployment process untii everyone is using the new "Plugin Installer"

See, https://opencpn.org/wiki/dokuwiki/doku.php?id=opencpn:developer_manual:ci-push-build-to-git
but you will need additional commands in Travis and Appveyor to push to github release tab.

I think I have a local master from your repository that should work for rebase.

@rgleason
Copy link
Contributor

rgleason commented Sep 27, 2019

Perhaps I should add, I have been deploying squiddio builds to here
https://github.com/rgleason/squiddio_pi/releases

This is where the recent published download plugins have been taken from.

Look at the appveyor.yml file (Windows) and the .travis.yml file for the most recent version 1.0.10
https://github.com/rgleason/squiddio_pi/blob/master/.travis.yml
https://github.com/rgleason/squiddio_pi/blob/master/appveyor.yml

For an example of what works. I can send you a listing of the git command and process to deploy using a pushed tag if that will help.

I hope you can get it working.

@mauroc
Copy link
Owner

mauroc commented Sep 28, 2019

thanks Rick - the latest examples you sent me are really helpful. My problem with travis
is that while it builds correctly,, I get the message at the end:

Skipping a deployment with the releases provider because this is not a tagged commit

I tried following the instructions in the manual to tag create a tag :

git tag v1.0.5
git push origin v1.0.5

right before my git commit and git push statements, so I am at a loss....
Do you know what I am doing wrong?

@mauroc
Copy link
Owner

mauroc commented Sep 28, 2019

never mind my last question: it looks like the

git tag
git push tag commands 

need to be issued after the last commit/push.

@rgleason
Copy link
Contributor

You've got it! Sorry I did not see your question earlier. Congratulations.

I have tried to do a rebase tonight, following the iterative nature of this process as carefully as I could, without knowing wheather I am doing it right or not. The rebase does not yet build in appveyor or travis. I may let Jon do it, but will try a little more.

@leamas
Copy link
Contributor

leamas commented Sep 28, 2019

I have set up a travis-ci.org account and was able to successfully build the 3 linux
distis + OSX using the .travis.yml file I believe @leamas had put in the mauroc/squiddio_pi repo

I havw actually added two independent build setups: One for travis-ci.org (.travis.yml) and one for circleci.com (.circleci/config). Having two setups was some redundancy in my PR, but turns out to be useful both here and in the radar_pi case.

My suggesiion: @mauroc sets up yet another account on circleci.com and enables also circleci
builds. From that point we treat these builds as a "plugin installer" ones. When it comes to travis, we let @rgleason use exactly what he has in his PR, disregarding any changes I have done to this setup.

One conflict is in appveyor.yml. What I did in the radar_pi case was to use "my" version of this file, but with @rgleason's deployment as the deployment section ("my" deployment is part of the build)

However, I am confused as to the proper place to deploy the packages.

I have placed them on cloudsmith.io after first trying bintray. It's a nice service which fulfills the objectives. However, it doesnt really matter where they go from the perspective of the installer. The dependencies are basically squiddio-plugin.xml.in (the dowmnload url) and the ci/ upload scripts.

@rgleason
Copy link
Contributor

rgleason commented Sep 28, 2019

The wiki page ci builds has the terminal statement to use with travis-ci. As a winows user, petri (canne) helped me a lot installing travis ci in a virtualbox. Hopefully you already have it installed?

Use travis ci to create an encryption, that you should put in after. "Secure: "...."

@mauroc
Copy link
Owner

mauroc commented Sep 28, 2019

@leamas of course, I have encrypted all the other tokens/keys but this one slipped. My bad. In any event, I have revoked it, and I don't plan on using Bintray anyway.

So here's where I am:

  • I have created the process to build and deploy to Github releases using my own travis-ci and appveyor accounts. (I have used the corresponding .yml files from Ricks repo in an effort to keep things somewhat consistent). Will we continue to support downloading packages manually from Github even after the Installer is functional?
  • I have created a circleci account. It looks like the packages were built successfully here, (though I did not see the Mac OS pkg).
  • I have created a cloudsmith account

What are the next steps? My understanding from here is that you have already updated squiddio_pi for the new Installer, so we should be pretty close ? Thanks!

@rgleason
Copy link
Contributor

Mauro
Not to be too picky, as what you got going is great, but shouldn't you have 7 assets in your release?

Dont know what is missing, but i think macos is one of them. Jon spoke of a pkg and a deb?

@mauroc
Copy link
Owner

mauroc commented Sep 29, 2019

right now I generate .deb (Debian), .exe (Windows), .pkg (Mac OS). I think I am missing Fedora and Flatpak. I will work on those but wanted to focus on the Installer first.

@leamas
Copy link
Contributor

leamas commented Sep 29, 2019

right now I generate .deb (Debian), .exe (Windows), .pkg (Mac OS). I think I am missing Fedora and Flatpak. I will work on those but wanted to focus on the Installer first.

You are actually generating flatpak, debian-10 and mingw/win32 on circle-ci, a MacOS .pkg build on travis and a msvc/win32 on appveyor.

One missing part is an MacOS .dmg build -- you will get that without further work if you just enable the MacOS builder on circle-ci. This requires a request + some manual handling on their side. For me this was very simple and fast.

Another missing part is a debian-9/trusty package. This is easily configured in travis, the original code had this is in place. The travis-build-debian script can build either Debian-9 or -10 out of the box.

You might even omit the travis Debian-10 package since it built in circe-ci anyway.

@rgleason
Copy link
Contributor

rgleason commented Sep 29, 2019

Leamas, thankyou for #39. When I get back I will look very carefully at it to try to understand how to rebase in the future. I was using the git command line to do this job and notepad++ to review and solve the conflicts flagged by rebase. Are there better tools? I have installed Atom and started using it a little.

So having done the rebase what should happen next?

My goals are as follows:

  1. Simplify and reduce cmake variables rather than always renaming.
  2. Have all variable settings up front in cmakelists.
  3. Use jon gough's improvements.
  4. Keep the appveyor and travis setups that allow us to create the 7 assets and push to github releases tab with a git push (tag).
  5. Get a standard simplified cmake files to be used for all plugins. Put them on the Dev manual wiki somehow (separate github opencpn repos for reference)?
  6. Learn how to impliment Leamas's system into other plugins so that the currently developed push tag to github release system can coexist with leamas system.
  7. Squiddio has been a very helpful example. Thankyou mauro and leamas.
  8. I would like to get original plugin authors able to create and deploy windows installer.exe to their github release tab, along with other os, as a first important step. I then can step back from doing this and my fork can become just a mirror.
  9. Get leamas plugin installer working on everyones opencpn.
  10. Retire the tag push to github release tab system

Can it be done? How long will it take?

Leamas you have paved the way!!!
Can you take us there?

@leamas
Copy link
Contributor

leamas commented Sep 30, 2019

My goals are as follows:
[ ]

Can it be done?

Anything can be done.

How long will it take?

Whatever estimate we do, multiplied with pi. Probably too long.

If I was to undertake something like this I would make it in smaller steps. The first could be your two first objectives covering the cmake setup variables. Try to isolate this change and get it accepted.

Leamas you have paved the way!!!
Can you take us there?

Sorry, no. My plate is already full.

@leamas
Copy link
Contributor

leamas commented Sep 30, 2019

right now I generate...

But the circleci builds are not deployed. The culprit is the missing CLOUDSMITH_API_KEY which is available i the cloudsmith web UI. That key should be made available in the circleci build environment, it's a simple setting in the circleci web ui.

BTW: The value of that key is not displayed by the script.

I think I am missing Fedora and Flatpak

Just drop fedora, it's not really useful. Fedora users can use flatpak, which already is built.

@rgleason
Copy link
Contributor

rgleason commented Sep 30, 2019

"Sorry, no. My plate is already full."
I guess I am not surprised after all you have done. Wish I had your skills.

Nevertheless, it would be helpful to have the push tag to github release working along with the new plugin installer, and to know how to impliment that for other pi and provide directions in the Dev wiki.

@mauroc
Copy link
Owner

mauroc commented Sep 30, 2019

The culprit is the missing CLOUDSMITH_API_KEY which is available i the cloudsmith web UI. That key should be made available in the circleci build environment, it's a simple setting in the circleci web ui.

I have my Cloudsmith API key...I just don't know where it goes in the plugin code, if it needs to be encrypted and if so using what encryption tool. (sorry it I am being thick. C++ is not my primary domain. I am mostly a backend web developer)

At some point this great document will have to be updated with the circleci and Cloudshmith stuff for other plugin developers. Right now it's mostly travis/appveyor and github.

@leamas
Copy link
Contributor

leamas commented Oct 1, 2019

I have my Cloudsmith API key...I just don't know where it goes in the plugin code

It doesn't go into the code. Just open the circleci web ui, point it to the squiddio project and open Preferences (gear symbol top-right). Here you have Environment Variables in the left pane. Just add CLOUDSMITH_API_KEY and you're done.

The trick is that the cloudsmith tool (a python script) invoked in circleci-upload.sh uses the environment variable directly, it doesn't have to be present on the command line.

At some point this great document will have to be updated

Indeed. Short-term, we also need to update the README at https://github.com/leamas/oesenc_pi.

@rgleason
Copy link
Contributor

rgleason commented Oct 1, 2019

Leamas,
Comparing cmakelists.txt

Jon had added in the first group
SET(PACKAGE_FILE_NAME "opencpn-plugin-squiddio")
which he used for the file names.
Is it a good idea to identify the program "opencpn" or is it not needed?

Mauro has
SET(PKG_API_VERSION "1.16")
which is correct for the current version of Opencpn and the git push to git release builds, but doesn't your Plugin Installer need API 1.17?
How is this going to be handled for testing and transition etc?

In cmakelists.txt what are these for? What does NVR stand for?
SET(PKG_RELEASE "1")
SET(PKG_NVR ${PACKAGE_NAME}-${PACKAGE_VERSION}-${PKG_RELEASE})

Is this the pointer to the file locations that Plugin Installer needs?
SET(PKG_BASE_URL
"https://dl.cloudsmith.io/public/alec-leamas/opencpn-plugins-unstable/raw/files")

below the include cmake/pluginsetup.cmake you have
set(PLUGIN_NAME squiddio-plugin-${PKG_TARGET}-${PKG_TARGET_VERSION})
PluginSetup.cmake is a new file that deals with OS and OS versions.
Is PKG_TARGET the OS intended?
PKG_TARGET_VERSION the OS version number intended?

@rgleason
Copy link
Contributor

rgleason commented Oct 1, 2019

Would it be easier to use Cloudsmith and its keys for the current deployment, rather than github release tab? Is a separate key or encryption required for each plugin?

@leamas
Copy link
Contributor

leamas commented Oct 1, 2019

Is it a good idea to identify the program "opencpn" or is it not needed?

When it comes to the installer, it's useless. Perhaps not in other contexts. A packaged plugin would certainly be named like opencpn-plugin-squiddio or so.

doesn't your Plugin Installer need API 1.17?

Not really, it (obviously) works with 1.16.

What does NVR stand for?

Name-Version-Release

Is PKG_TARGET the OS intended? PKG_TARGET_VERSION the OS version number intended?

Yes

Is this the pointer to the file locations that Plugin Installer needs?

yes

The basic requirement is that we have to have unique filenames when changing stuff A file name like squiddio-plugin-0.6.2-1_debian-10 should be interpreted like:

  • squiddio-plugin is indeed the name, used to distingush from other plugins.
  • 0.6.2 is the squiddio release number, probably tagged.
  • -1 represents the release. That is, if the packaging changes without changing 0.6.2 we get -2.
  • debian-10 represents the target OS and version plugin is built for (and linked with!)

By using all these parts in the filename we avoid name clashes when plugins are updated one way or another.

@rgleason
Copy link
Contributor

rgleason commented Oct 1, 2019

Jon had some good ideas for simplifying the variables. Maybe he can make a PR that does that that uses Leamas merged PR as a basis.

@leamas
Copy link
Contributor

leamas commented Oct 2, 2019

Would it be easier to use Cloudsmith and its keys for the current deployment, rather than
github release tab?

I think so, yes. Overall, the circleci-cloudsmith build chain has advantages:

  • circleci allows users to log in to failed builds. This is a major advantage compared to travis which
    basically just fails with a more or less cryptic message.
  • circleci is faster than travis, I have seen 20% to 100% faster builds
  • The travis command line is a ruby thing which requires ruby libraries. I consider this as a drawback
    compared to the cloudsmith cli application which is a regular python package available on pypi.org
  • Cloudsmith does not require any complicated key encryption, it uses a key in the build environment
    in a simple and secure way.
  • The cloudsmith administrative API offers a lot more compared to github.

In many ways, bintray is a viable alternative to cloudsmith. In the end, my conclusion is that cloudsmith is (marginally) better. Both are way ahead of github when it comes functionality.

is a separate key or encryption required for each plugin?

The key is per repository. No encryption is needed. In the end, it's a question about who controls the repo -- a central one for all plugins or a repo per plugin.

OTOH, this is just because you ask. I have no interest to push deployment solutions on anyone. As long as things can be found and downloaded I guess we are OK.

@rgleason
Copy link
Contributor

rgleason commented Oct 2, 2019

Leamas, thankyou! that is the direction I am headed. In addition to your list this one is the cincher for me:

Cloudsmith does not require any complicated key encryption, it uses a key in the build environment
in a simple and secure way.

It makes the files useful for anyone who sets up the their account without changes.

@rgleason
Copy link
Contributor

rgleason commented Oct 2, 2019

Leamas, is there an appropriate place where I can add the "opencpn-" to the filenames created for the current system and packages that get pushed to github release? ...or is this just to complicated and not worthwhile for our purposes?

@rgleason
Copy link
Contributor

rgleason commented Oct 3, 2019

Mauroc,
I am closing this and between Jon and I, perhaps we can do as Leamas suggests, make smaller PR's
that you can accept.
That's probably the best way to do it anyway.

@rgleason
Copy link
Contributor

rgleason commented Oct 3, 2019

Perhaps Jon Gough needs to close this. I don't see where to do this.

@rgleason
Copy link
Contributor

rgleason commented Oct 11, 2019

After force pulling mauro's most current master and comparing files with rgleason/od_fixes branch with my editor, I am finding that

  • .travis.yml - almost identical, no meaningful difference
  • appveyor.yml - almost identical, no meaningful difference
  • cmakelists.txt - identical now
  • Under "cmake" directory
  • pluginconfigure.cmake - match
  • plugininstall.cmake - match
  • version.h.in - match

pluginpackage.cmake - some differences, see below.

Line 174
master: FILES ${PREFIX_PARENTLIB}/lib${PACKAGE_NAME}.dylib
od_fixes: FILES ${PREFIX_PARENTLIB}/libsquiddio_pi.dylib

Line 179
master: "${CMAKE_INSTALL_PREFIX}/bin/OpenCPN.app/Contents/PlugIns/lib${PACKAGE_NAME}.dylib"
od_fixes: "${CMAKE_INSTALL_PREFIX}/bin/OpenCPN.app/Contents/PlugIns/libsquiddio_pi.dylib"

Line 187
master: --volname "${PACKAGE_NAME} Installer"
od_fixes: --volname "squiddio_pi Installer"

Line 191
master: DEPENDS ${CMAKE_INSTALL_PREFIX}/bin/OpenCPN.app/Contents/PlugIns/lib${PACKAGE_NAME}.dylib
od_fixes: DEPENDS ${CMAKE_INSTALL_PREFIX}/bin/OpenCPN.app/Contents/PlugIns/libsquiddio_pi.dylib

(Copied the above differences from Master branch over to od_fixes branch)

@rgleason
Copy link
Contributor

rgleason commented Oct 11, 2019

Comparing squiddio_pi.cpp and squiddio_pi.h
The two changes made by Jon in the master to fix blank icons are good.
(Copied those changes over to od_fixes)

After making these changes to od_fixes branch,
It still compiled without errors.

@jongough jongough closed this Oct 11, 2019
@jongough jongough deleted the od_fixes branch October 11, 2019 03:33
@rgleason
Copy link
Contributor

rgleason commented Oct 11, 2019

Ok so you're working on it.
Now I see you deleted the PR and deleted rgleason:od_fixes branch 14 hours ago.

@rgleason rgleason restored the od_fixes branch October 12, 2019 03:23
@rgleason rgleason deleted the od_fixes branch April 9, 2020 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants