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

[Bug] Some AddOns broken for GRASS 8 due to their API change #616

Closed
3 tasks done
pesekon2 opened this issue Sep 29, 2021 · 5 comments · Fixed by #637
Closed
3 tasks done

[Bug] Some AddOns broken for GRASS 8 due to their API change #616

pesekon2 opened this issue Sep 29, 2021 · 5 comments · Fixed by #637
Assignees
Labels
bug Something isn't working

Comments

@pesekon2
Copy link
Contributor

pesekon2 commented Sep 29, 2021

After the branch for GRASS 8 will be finally created (see #528), fix addons affected by this change.

The PR will tag original authors of the addons as reviewers to give them a chance to state their opinion about changes.

So far those addons were discovered to need some care:

  • i.sentinel.coverage - caused by a change in v.db.select
  • v.habitat.dem - caused by a change in v.db.select
  • r.valley.bottom (temporarily fixed by an if statement) - going to have a PR for grass7 and grass8 branch - caused by a change in r.neighbors
@pesekon2 pesekon2 added the bug Something isn't working label Sep 29, 2021
@pesekon2 pesekon2 self-assigned this Sep 29, 2021
@wenzeslaus
Copy link
Member

Finally, the grass8 now exists and it is the default branch in the grass-addons repo.

@veroandreo
Copy link
Contributor

veroandreo commented Dec 3, 2021

Finally, the grass8 now exists and it is the default branch in the grass-addons repo.

This means that installing addons for UNIX like systems is again broken in grass 7x, right? Couldn't we have waited to make it default until we actually release grass 8, to avoid these breakings?

@wenzeslaus
Copy link
Member

This means that installing addons for UNIX like systems is again broken in grass 7x, right?

g.extension r.sun.hourly works for me with 7.8.6 and with the main branch (v8.2dev). Is there some issue you see now?

Couldn't we have waited to make it default until we actually release grass 8, to avoid these breakings?

If it doesn't work, the problem is with the code. The new code should always use the appropriate branch, not the default branch.

@veroandreo
Copy link
Contributor

This means that installing addons for UNIX like systems is again broken in grass 7x, right?

g.extension r.sun.hourly works for me with 7.8.6 and with the main branch (v8.2dev). Is there some issue you see now?

Couldn't we have waited to make it default until we actually release grass 8, to avoid these breakings?

If it doesn't work, the problem is with the code. The new code should always use the appropriate branch, not the default branch.

Apologies for the noise, I asked based on what happened last time. But now I tested and it works just fine. Great! :)

@wenzeslaus
Copy link
Member

...what happened last time...

Yes, last time it happened because Git/GitHub support was not well implemented in g.extension, but when fixing it, @ninsbl worked hard to make it robust for the next change.

pesekon2 added a commit that referenced this issue Dec 6, 2021
IvanMarchesini pushed a commit to IvanMarchesini/grass-addons that referenced this issue Feb 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants