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

$ISISDATA under version control #351

Closed
jlaura opened this issue Jul 13, 2018 · 16 comments
Closed

$ISISDATA under version control #351

jlaura opened this issue Jul 13, 2018 · 16 comments
Labels
automatically_closed Issue closed by a bot due to inactivity enhancement New feature or request

Comments

@jlaura
Copy link
Collaborator

jlaura commented Jul 13, 2018

This is piggybacking off of #350:

When I look at the base data area, I see that the translations are ~420k of test files, applications are 28K of text files, and icons are ~3MB of pngs. Why aren't we keeping these under version control with the source? If the concern is the pngs, we can convert to SVG.

If this were the case, @rbeyer could submit a PR (or we could PR in the fix just as easily) to address #350.

@rbeyer
Copy link
Contributor

rbeyer commented Jul 13, 2018

Yeah, this is true. If it were here, I'd have just submitted a pull request (PR) instead of an issue.

GitHub can take PNGs just fine, sure the version control thing isn't served well, but bits are bits to git.

@jlaura
Copy link
Collaborator Author

jlaura commented Jul 13, 2018

@jessemapel @ihumphrey-usgs I can put in a PR that gets the above files under VC. This can happen in the CMake branch even.

An unconstrained search of translations looks like the changes would be relatively well contained for the .trn, .typ, and .def files.

In the interim to making something like this happen, can we get the base data updated?

@jlaura jlaura added the enhancement New feature or request label Jul 13, 2018
@jessemapel
Copy link
Contributor

Moving the translation files into the source code will also need to account for custom data areas. It could be that we actually don't want people using custom data areas for translations and configs.

@jessemapel
Copy link
Contributor

Also, this is just a general ISIS recommendation and not directly related to cmake so I'm going to move it to a redmine issue.

@jlaura
Copy link
Collaborator Author

jlaura commented Jul 17, 2018

Can you link to the internal issues here (and #350)? Thanks!

@jessemapel
Copy link
Contributor

jessemapel commented Jul 17, 2018

Redmine issue

@kberryUSGS
Copy link
Contributor

@jlaura @jessemapel As an additional complication, right now we store kernels in the $ISIS3DATA area. We probably don't want to put those under version control.

@kberryUSGS
Copy link
Contributor

@jessemapel Can you update the link to point to the external fixit (https://isis.astrogeology.usgs.gov/fixit) instead of the internal-only link?

@jessemapel
Copy link
Contributor

Oops thank you!

@jlaura
Copy link
Collaborator Author

jlaura commented Jul 17, 2018

@kberryUSGS Agreed, the kernels should not be under standard git version control (but should perhaps be managed using git-lfs). This issue is requesting that the ISIS team look at separating the plaintext components out of the $ISISDATA area so that (1) they can be put under version control (are they now?) and (2) so that users can submit PRs to make improvements to this piece of the system.

@jlaura
Copy link
Collaborator Author

jlaura commented Jul 17, 2018

@jessemapel why are we closing this - this issue is still open?

@jessemapel
Copy link
Contributor

@jlaura I don't want to have discussion happening in two different places, here and on the redmine ticket.

I also don't want this type of thing requested through GitHub issues, so I don't want to leave this open. We can eventually move stuff like this to being done through Git issues, but right now we've committed to this type of things being discussed on our redmine site.

@jlaura
Copy link
Collaborator Author

jlaura commented Jul 18, 2018

@jessemapel That work here. One problem - an external user has no way of knowing this. Github issues are enabled (which is awesome) and no information or link to any external issue tracker exists.

@jessemapel
Copy link
Contributor

I'll probably put in a PR this week to put some of that information in our Readme. It's rather empty at the moment.

@jlaura
Copy link
Collaborator Author

jlaura commented Jul 18, 2018 via email

@ascbot
Copy link
Contributor

ascbot commented Sep 1, 2020

I am a bot that cleans up old issues that do not have activity.

Happy Birthday to this issue! 🎂

Unfortunately, this issue has not received much attention in the last 12 months. Therefore, I am going to close it. Please feel free to reopen this issue or open a new issue sometime in the future. If this issue is a bug report, please check that the issue still exists in our newest version before reopening.

@ascbot ascbot added the automatically_closed Issue closed by a bot due to inactivity label Sep 1, 2020
@ascbot ascbot closed this as completed Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automatically_closed Issue closed by a bot due to inactivity enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants