sailfishos-chum / main

Documentation and issue tracker for the SailfishOS:Chum community repository
https://build.merproject.org/project/show/sailfishos:chum
MIT License
26 stars 4 forks source link

Enhance Metadata.md to address issue #78 #80

Closed Olf0 closed 2 years ago

Olf0 commented 2 years ago

@rinigus,

  1. Is this really correct? I added the sentence, "If Repo: is not set or the metadata for SailfishOS:Chum is completely missing, the URL provided by the URL: field in the spec file preamble is used instead." You wrote that the "RPM SPEC URL [is] used as a fallback for Url/Repo", but AFAICS in chumpackage.cpp m_url is preset with the URL: field of the spec file preamble (in line 118) and then used as the default value for the Homepage: field (in line 210). But the m_repo_url is left empty when not set (see line 203) before entering projectgithub.cpp or projectgitlab.cpp; what these two do is hard to grasp for me as a former Shell and C programmer.
  2. For the field DeveloperName: the comment states If not set, and a GitHub repository is set, then the name will be automatically retrieved. Note that such automatic retrieval is not supported for GitLab repositories.
    • a. The statement "a GitHub repository is set" is imprecise: By which field(s)? Presumably as Repo:-URL, but with or without the fallback mechanism discussed in 1., above?
    • b. This automatic retrieval of the name is only described for the field DeveloperName:, but not for the field PackagerName:. I think they should be handled then same, i.e., If not set, and a GitHub repository is setas PackagingRepo:, then the name will be automatically retrieved. Note that such automatic retrieval is not supported for GitLab repositories. Is this already the case?
    • c. For obtaining a name at GitLab, as a fallback the URL might be parsed, though this provides the login name, not the pretty name, see, e.g.: https://gitlab.com/Olf0/sailfishX Although the pretty name can be extracted as the first string on a GitLab user page proper (when ignoring GitLab's top menu bar), see, e.g.: https://gitlab.com/Olf0
rinigus commented 2 years ago

@Olf0: I'll reply few days later, sorry. Will check the code as well

rinigus commented 2 years ago

@Olf0:

1: yes, it is correct. Loop at https://github.com/sailfishos-chum/sailfishos-chum-gui/blob/main/src/chumpackage.cpp#L215 runs through {m_repo_url, m_url, m_packaging_repo_url} and if m_repo_url is not set it will try m_url next. That URL is used in constructor for GitHub/GitLab projects (if they fit the scheme as tested by ::isProject) and through that would support corresponding features (issues, releases)

2a,b: If any of the three is set ({m_repo_url, m_url, m_packaging_repo_url}), developer name would be set from that if needed. Now, indeed, packager name is not currently set if we query m_packaging_repo_url. Surely can be done, just wasn't as a part of the spec when I implemented it. Can be considered as a bug / missing feature

2c: We can consider doing it, indeed.

Olf0 commented 2 years ago

These were quick "few days" (just one)! :wink:

1: yes, it is correct.

Thank you for the clarification.

2a: If any of the three is set ({m_repo_url, m_url, m_packaging_repo_url}), developer name would be set from that if needed.

For me it seems to be not O.K. to extract the developer's name from the m_packaging_repo_url, even as a second fallback.

2b: Now, indeed, packager name is not currently set if we query m_packaging_repo_url.

In contrast to the behaviour you described for 2a, this appears to be logical.

Surely can be done, just wasn't as a part of the spec when I implemented it. Can be considered as a bug / missing feature.

O.K. Which of the following two options do you prefer?

  1. Leave everything as it is (code and this PR), and mark this PR as "ready for review"?
  2. Address 2a & 2b in the code, while I extend this PR to cover this behaviour.

2c: We can consider doing it, indeed.

In general I do not like web-page "scraping", because web-pages are often altered. But in this case, I believe that the pretty name will very likely always be right below the avatar picture, so this is a sustainable solution.

Filed as issue #81.

Olf0 commented 2 years ago

@rinigus, before we get side-tracked into point 2c / issue #81 even more, the main open question for this PR is (WRT points 2a & 2b):

O.K. Which of the following two options do you prefer?

  1. Leave everything as it is (code and this PR), and mark this PR as "ready for review"?
  2. Address 2a & 2b in the code, while I extend this PR to cover this behaviour.
rinigus commented 2 years ago

@Olf0: sorry for delays. I think it would make sense to proceed with as it is. In general, with Chum, we would have to work taking into account that Jolla can pull the plug on OBS in May. So, before we invest a lot of time into improvement of it, would be nice to get OBS status reaffirmed. On the other side, we shouldn't let it rot either as that would be a mistake as well. Would have to balance those aspects :)

Olf0 commented 2 years ago

@Olf0: sorry for delays. I think it would make sense to proceed with as it is.

O.K., done (sorry I had forgotten to do this).

In general, with Chum, we would have to work taking into account that Jolla can pull the plug on OBS in May.

Oh, I missed that. Where does that May deadline come from? I had the impression that (after the heated discussion at FSO) the Sailfish-OBS will be maintained until Jolla provides a viable alternative, which I expect to come in "Jolla time": years or never.

Olf0 commented 2 years ago

I found it. Thank you for posing the question to the IRC community meeting 2022-05-12.

Oh, I missed that. Where does that May deadline come from? I had the impression that (after the heated discussion at FSO) the Sailfish-OBS will be maintained until Jolla provides a viable alternative, which I expect to come in "Jolla time": years or never.

As expected, the answer is vague and tardy, but the undertone appears to be negative. :disappointed:

rinigus commented 2 years ago

Indeed, no clear reply and we have to search for undertones without being given +/- and their contributions to decision.

Olf0 commented 2 years ago

Still …

I think it would make sense to proceed with as it is.

Or is there anything holding you back from merging this (and also PR #79)?

rinigus commented 2 years ago

@Olf0: sorry, just forgot to do so.